Sign in to follow this  
gretty

strnstr() Problem: Using C string altering functions

Recommended Posts

Hi I am trying to use the function [b] strnstr() [\b] (from string.h) to check that the last 3 characters of a [b]const char*[\b] variable equals [b]"\\*"[\b].

But I am getting some problems:

- First off my compiler is telling me there is no function [b]strnstr()[\b]? Am I importing the correct library to use this function(string.h)?

- Second, I always get an error when I use a [b]char*[\b] variable as the 1st parameter in strstr() but when I use a [b]const char*[\b] variable is seems ok? My compiler tells me that I have "an invalid conversion from char* to char"?

- Third, I am trying to format a string (which is a path to a folder directory) so I can use it in FindFirstFile(). So I need to add this ([b]"\\*"[\b]) to the end of the directory. Is [b]"\\*"[\b] considered a character or a string? You know how "\d" is considered a character. If [b]"\\*"[\b] is a character this could be one of the reasons I am getting an error & I should be using strchr().


My code that is giving me errors:

#define WINVER 0x0502

#include <windows.h>
#include <iostream>
#include "conio.h"
#include <string.h>

using namespace std;


const char* formatFilePath( const char* fileName )
{
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).


char* formatPath, specLocale;
size_t fileNameLen = strlen( fileName );

specLocale = strnstr( fileName, "\\*", fileNameLen - 4 ); // the compiler says this function doesnt exist & wont let me pass a char* variable as the 1st param?

// if special identifier is not present in char array
if ( specLocale == NULL ) // compiler says I cannot check that a char* == NULL ...thats wierd??
{
return "Invalid Path";
}


if ( strncmp( fileName, "\\*", 2 ) != 0 )
{
return strcpy( fileName, "\\*" ); // OR strcpy( fileName, "\\*", 4 );
}

return "Invalid Path";
}


const char* formatFilePathEx( string fileName )
{
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).
// Uses std string class functions to format string


char* formatPath;
size_t specLocale = fileName.rfind( "\\*" );


if ( specLocale == string::npos )
{
fileName += "\\*";
}
else if ( (fileName.length - specLocale) != 3 ) // ps: why do I get an error here because I am
// doing a (int - size_t) != int ?
{
// fileName contains special locale("//*") but its NOT
// at the end of the string(where it should be)
fileName = "Invalid Path";
}


return fileName.c_str();

}


int main()
{

system("PAUSE");
return 0;
}





Share this post


Link to post
Share on other sites
Quote:
Original post by gretty
Hi I am trying to use the function [b] strnstr() [\b] (from string.h) to check that the last 3 characters of a [b]const char*[\b] variable equals [b]"\\*"[\b].


Huh? strnstr doesn't exist. What documentation are you using? Did you mean strncmp?

Share this post


Link to post
Share on other sites
Just some comments in no particular order (please forgive the brevity, in a bit of a hurry):

  • Tags on GDNet are closed with a / not a \

  • strnstr is not a standard C or C++ library function. (It is, AFAIK, only widely available on FreeBSD variants, which may or may not include Mac OS X, I can't recall offhand.)

  • strnstr does not work the way you are using it; the numeric parameter is not a start offset, it's a maximum length. In other words, it won't search past the index you provide.

  • The correct operation would be to use strcmp(fileName + fileNameLen - 3, magic) since you want to compare 3 characters from the end of the string.

  • When discussing compile errors, post the precise exact and unaltered text of the error, as well as the code that triggers it. As such we have no idea what the problem could be.

  • The sequence "foo" is a string. The sequence "f" is a string. The sequence "\n" is a string. The sequence 't' is a character, as is '\n'. The differentiation is in the quotes, not the length.

  • The sequence "\\*" is not three characters, but two. The \ is an escape character used to denote special bytes, as in \n and so on. \\ is the way to specify the single character \. If you want to compare for the actual string "\\*" you need to use the code expression "\\\\*" to get two \ characters and a *. Otherwise, if you only want to look for the string "\*", you can use the code expression "\\*" as you posted, but remember that that is two characters not three.

  • You are using C++, which means there is no justifiable reason whatsoever for using C strings. Use std::string uniformly, it'll eliminate half a dozen bugs and potential security flaws in your code.

  • Don't pass heavy objects like std::string by value. Use a const reference instead (i.e. const std::string&).

  • Your code has a massive error, in that you use a copy of the fileName string, return its const char* conversion, and then the object goes out of scope when the function returns; this means that your pointer will be garbage. Again, use strings uniformly, this'll eliminate the problem.



That's all that comes to mind offhand, but there may yet be other things I've missed.

Share this post


Link to post
Share on other sites
Thanks for the replies

I was going off this site for strnstr()
http://www.codecogs.com/reference/c/string.h/strstr.php


Using strcmp() & ur code example... is it advancing the char pointer to the 3rd last char of the char (array/pointer) then checking to see if that equals the "\\\\*"?

strcmp(fileName + fileNameLen - 3, "\\\\*"); ?


I am using c strings & its altering functions just to learn but I'll keep it in mind to stick to std::string. I thought that maybe using c_strings could use less memory(but it may be negligible).

Also, when you say I have a major error with my function
char* formatFilePathEx( string fileName ) I kindof understand what you mean. But I am returning a char* because its the only string I can use in the function FindFirstFile(). And if I change it to
char* formatFilePathEx( const string& fileName ) wont that mean that I wont be able to add "\\n" to the end of fileName? Maybe I should return a LPTSTR?

Share this post


Link to post
Share on other sites
Thanks for the advice I have implemented the changes but I am getting 2 smallish :P errors which I dont understand how to fix.

The first problem I have tried casting to a char* but that doesn't work

I have commented the compiler errors & where the 2 errors occur(still dont know if I am legally/properly converting a string to const char* but I think it will work):


const char* formatFilePath( char* fileName )
{
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).


size_t fileNameLen = strlen( fileName );
char* specLocale = strcmp( fileName + fileNameLen - 3, "\\\\*" ); // Compiler error "Invalid conversion from 'int' to 'char*' "


// if special identifier is not present in char array
if ( specLocale == NULL )
{
strcpy( fileName, "\\\\*" ); // OR strcpy( fileName, "\\*", 4 );
}

return fileName;
}


const char* formatFilePathEx( string &fileName )
{
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).
// Uses std string class functions to format string


char* formatPath;
size_t specLocale = fileName.rfind( "\\\\*" );


if ( specLocale == string::npos )
{
fileName += "\\\\*";
}
else if ( (fileName.length - specLocale) != 3 ) // Compiler error: "Invalid use of member (did you forget the '&' ?)"
{
// fileName contains special locale("//*") but its NOT
// at the end of the string(where it should be)
fileName = "Invalid Path";
}


return fileName.c_str();

}


Share this post


Link to post
Share on other sites
Quote:
Original post by gretty
Using strcmp() & ur code example... is it advancing the char pointer to the 3rd last char of the char (array/pointer) then checking to see if that equals the "\\\\*"?

strcmp(fileName + fileNameLen - 3, "\\\\*"); ?


Yes, that's what it does.


Quote:
I am using c strings & its altering functions just to learn but I'll keep it in mind to stick to std::string. I thought that maybe using c_strings could use less memory(but it may be negligible).


You're talking about a very small number of bytes here. The overhead is totally irrelevant in 99.99% of cases, because the ease of making mistakes and doing things Wrong™ with C-style strings is prohibitively risky.


Quote:
Also, when you say I have a major error with my function
char* formatFilePathEx( string fileName ) I kindof understand what you mean. But I am returning a char* because its the only string I can use in the function FindFirstFile(). And if I change it to
char* formatFilePathEx( const string& fileName ) wont that mean that I wont be able to add "\\n" to the end of fileName? Maybe I should return a LPTSTR?


You need to re-read what I said; you missed all the important parts.

Also, LPTSTR is char*. There's no difference.


Quote:
Original post by gretty
Thanks for the advice I have implemented the changes but I am getting 2 smallish :P errors which I dont understand how to fix.

The first problem I have tried casting to a char* but that doesn't work

I have commented the compiler errors & where the 2 errors occur(still dont know if I am legally/properly converting a string to const char* but I think it will work):

*** Source Snippet Removed ***


*sigh*

  • Why did you ignore the part about using std::string? Your code is riddled with bugs and mistakes because of your insistence on mixing C-strings and string objects. Use strings uniformly.

  • strcmp does not return a char*, which is what your compiler is trying to tell you. It returns an int. If you are not familiar with a function, you should at least look it up on Google or something before using it. Never blindly throw a function at your code without knowing how it works.

  • You are using strcpy incorrectly. Again, look things up before using them.

  • Your second implementation is closer to workable (but not there yet); but why do you have two? Don't duplicate code unnecessarily.

  • Reread that bit earlier about the massive problem in your code. You're making a critical mistake here that you really need to understand before moving on. If you need I can provide more details, but until we get that squared away you'll continue to run into some potentially really evil bugs.

Share this post


Link to post
Share on other sites
Quote:
•Reread that bit earlier about the massive problem in your code. You're making a critical mistake here that you really need to understand before moving on. If you need I can provide more details, but until we get that squared away you'll continue to run into some potentially really evil bugs.


Ok I reread what you said & have implemented some changes. Ps, yeah I was confusing myself as to what strcpy() does, changed it to strcat() :P. But more details would be helpful :)

So I know what ur saying that once I return a pointer to some data that is local it will be deleted (unless I declare it as dynamic) & other problems with my code. My ultimate goal is to try to return a c_string in a function because I want to know I can do it(I've never done it before) but is this a big no no or not possible?

So 1st is the updated code, please if there are more holes you can see I'd like to know:

const char* formatFilePath( char* fileName )
{
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).


size_t fileNameLen = strlen( fileName );
int specLocale = strcmp( fileName + fileNameLen - 3, "\\\\*" );


// if special identifier is not present in char array
if ( specLocale != 0 )
{
strcat( fileName, "\\\\*" );
}

return fileName;
}


string formatFilePath( string &fileName )
{
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).
// Uses std string class functions to format string


size_t specLocale = fileName.rfind( "\\\\*" );


if ( specLocale == string::npos )
{
fileName += "\\\\*";
}
else if ( (fileName.length() - specLocale) != 3 )
{
// fileName contains special locale("//*") but its NOT
// at the end of the string(where it should be)
// fileName = "Invalid Path";
return NULL;
}


return fileName;

}




And 2nd, here is an attempt to take a string as a parameter &...returns a char*, now dont sigh again :P but I am only doing this to learn I HEED your advice I just like to experiment & see if I have the skills to do this, so are you able to tell me if what I am doing is possible & if it has any holes. I reakon that theoretically copying the string into a c_string initally may avoid the problem of going out of scope:

const char* formatFilePathEx( string &fileName )
{
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).
// Uses std string class functions to format string


char* result;
size_t specLocale = fileName.rfind( "\\\\*" );

strcpy( result, fileName.c_str() );


if ( specLocale == string::npos )
{
//fileName += "\\\\*";
strcat( result, "\\\\*" );
}
else if ( (fileName.length() - specLocale) != 3 ) // Compiler error: "Invalid use of member (did you forget the '&' ?)"
{
// fileName contains special locale("//*") but its NOT
// at the end of the string(where it should be)
// fileName = "Invalid Path";
return NULL;
}


return result;

}


Share this post


Link to post
Share on other sites
Quote:

My ultimate goal is to try to return a c_string in a function because I want to know I can do it(I've never done it before) but is this a big no no or not possible?

In C++, this is done by return a std::string and using its c_str() member function. In C (or C++ without std::string), there are a couple of ways of approaching this, but they all have pitfalls.

  • The simplest way is to return a dynamically allocated block, e.g. with malloc() or new[]. This has the downside that the caller needs to free the block.

  • Return a pointer to static or global memory. This is easy and the caller needs to do no special work. However, there will be some fixed upper length on the data and this is not thread safe

  • Get the caller to pass in a buffer and a length, and write to it. This is the most common version. Its downsides include buffer,length mismatches by the client and some upper size that your code now has to handle.

Your first function (in C) assumes the caller has allocated enough space for the extra characters. There is no way to test this without asking the caller to pass a "max buffer size" parameter.

For your first snippet (in C++), you cannot return NULL for a std::string. This will typically crash, or trigger an assertion. Good unit tests would have found this bug for you. Arguably the condition you are checking for is rare enough to be exceptional, and you might consider using an exception to deal with this odd event. Pass objects by const reference unless you really want to modify them in the function.

Your second code tries to strcpy to an invalid pointer, so is incorrect.

std::string handles string length and buffer size correctly in all cases, this is why it is easier to get things working with std::string.

Share this post


Link to post
Share on other sites
Ah I get it now! :D

Thanks for all your help & patience

I have gone with your last suggested method & now I see that there isn't a need to return anything in the function aswell.

Its late here so I am going to fiddle around with c_strings tomorrow, I shd be studying for exams but this is more fun :P

Here is how I have interpretted what you said, as you said I cant really determine the size of the buffer array (inside the function) so thats one of many ways the function could fail, but it works for most general situations right now:

char* formatFilePath( char* fileName, unsigned int bufferLen )
{
// Pre :
// Post: Check that a file path is correctly formatted in
// order to be used in win32 file functions
// (FindFirstFile, etc.).


size_t fileNameLen = strlen( fileName );

if ( fileNameLen <= 2 || bufferLen < fileNameLen+3 )
{
cout << bufferLen << ", " << fileNameLen << endl;
cout << "invalid \n";
return NULL;
}

int specLocale = strcmp( fileName + (fileNameLen - 2), "\\*" );


// if special identifier is not present in char array
if ( specLocale != 0 )
{
strcat( fileName, "\\*" );
}


return fileName;
}

Share this post


Link to post
Share on other sites
If you already modify the string in your function, why return a pointer to the same string? Doesn't accomplish anything.

I know you said you're trying to return a C-string from a function, but why? Programming is not about picking an arbitrary technical goal and then contorting yourself until you get there; programming is about having a usability goal and doing what it takes technically to achieve it. You seem to be floundering because you're stuck on a pointless goal with no real significance or guiding purpose. Although your function may work, I'll bet you a couple of beers that you have major bugs in the code that calls it.


I generally try to avoid writing code for people, but in this case, I wanted to illustrate how much simpler your life could be if you quit trying to do things the wrong way round.

std::string FormatFilePath(const std::string& filename)
{
std::string ret(filename);
if(ret.length() < 2 || ret.substr(ret.length() - 2) != "\\*")
ret += "\\*";
return ret;
}

// Or if you prefer mutating your strings in place:
void FormatFilePath(std::string& filename)
{
if(filename.length() < 2 || filename.substr(filename.length() - 2) != "\\*")
filename += "\\*";
}


Share this post


Link to post
Share on other sites
Quote:
Original post by ApochPiQ
std::string FormatFilePath(const std::string& filename)
{
std::string ret(filename);
if(ret.length() < 2 || ret.substr(ret.length() - 2) != "\\*")
ret += "\\*";
return ret;
}

// Or if you prefer mutating your strings in place:
void FormatFilePath(std::string& filename)
{
if(filename.length() < 2 || filename.substr(filename.length() - 2) != "\\*")
filename += "\\*";
}


Or even better:


#include <boost/algorithms/string.hpp>
std::string FormatFilePath(const std::string& filename)
{
if (boost::ends_with(filename, "\\*")
return filename;
else
return filename + "\\*";
}

Share this post


Link to post
Share on other sites
That is pretty close, but it will fail in some cases.

For instance, here is a case where there is no room in the buffer, but we don't need any additional room because the string is already present.

int main()
{
char fileName [] = "C:\\foo\\bar\\*";
char *result = formatFilePath(fileName, sizeof(fileName)); // NB: see end of post
printf("Filename is: %s\n", result ? result : "failed!");
}


This fails in your current code.

And what about the case where the user wants to search the current or parent directories ("." and "..")? Why can't these work?

You have a lot of magic numbers in there, you should see if you can derive them from known quantities (such as the length of the string \\*).

Write a test program that you expect to work. This should include all kinds of corner cases you can think of:

#include <cassert>

template<int N>
void pass(int run, char (&buffer)[N])
{
char *result = formatFilePath(buffer, N);
if(buffer != result)
{
std::cout << "Fail(" << run << ") expected " << buffer << " got \'" << (result ? result : "<NULL>")<< "\'\n";
}
else
{
std::cout << "Pass(" << run << ")\n";
}
// assert that the end of buffer == \\*
}

template<int N>
void fail(int run, char (&buffer)[N])
{
char *result = formatFilePath(buffer, N);
if(0 != result)
{
std::cout << "Fail(" << run << ") expecting NULL got \'" << result << "\'\n";
}
else
{
std::cout << "Pass(" << run << ")\n";
}
}

int main()
{
char tooSmall[] = ".";
fail(1, tooSmall);

char currentDirectory[64] = ".";
pass(2, currentDirectory);

char noModification[] = "C:\\foo\\bar\\*";
pass(3, noModification);

char needsAddition[64] = "C:\\foo\\bar";
pass(4, needsAddition);

char illegal[64] = "C:\\foo\\*\\bar";
fail(5, illegal);
}


This is a simple example of a unit test. I'm sure you can think of a few more examples, for instance if the user passes NULL or an empty string "", that you don't want the program to crash.

There are frameworks out there for more robust unit testing that can be performed as part of the build process. But even a simple example like the above is enough to test

The strange looking template functions at the top use a special technique to discover the size of the buffer. You cannot use this technique on pointers or on arrays passed to a function (as these decay to pointers).

* Don't use sizeof() for buffer lengths unless you truly understand how arrays decay to pointers. Note that the template approach given above is safe but can result in lots of template expansions.

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
That is pretty close, but it will fail in some cases.


Me's slightly confused about the addressee of your post.

Share this post


Link to post
Share on other sites
I wrote most of it before lunch (Irish time) and at the time the directly preceding post was the OPs. I posted it after lunch, but I should have maybe clarified this point.

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
I wrote most of it before lunch (Irish time) and at the time the directly preceding post was the OPs. I posted it after lunch, but I should have maybe clarified this point.


Ah, see, thanks for clarifying :)

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this