strtok/strtok_r woes

[kbhat]

I had written some code using strtok_r for parsing strings with two
"levels" of delimiters. The code was working like a charm, until it
suddenly broke one day. You see, I was applying strtok on the input
string itself, and as long as a variable string was passed to the
function, everything was hunky dory. Then one day somebody decided to
pass a constant string to my code ---- and everything came collapsing
down like a domino.

I believe that this is a pitfall into which many programmers may fall.
I have now changed the signature of my function to treat the input
string as a constant string, and I now making a local copy of the
string and operating on that. Of course, I now have to ensure that I
free up the dynamically allocated memory.
Ciao
KB

[I have posted my source listing to comp.sources.d]

[kbhat]

Modified source listing for my program using strtok_r in which I make a
local copy of the input string and operate strtok on that
// Program that parses strings of the form:
// Name, SS#, Emp#, Other#::Name, SS#, Emp#, Other#::Name, SS#, Emp#,
Other#
// and extracts the name and other attributes associated with an
employee
#include <stdio.h>
#include <iostream.h>
#include <fstream.h>

#include <list.h>

extern "C" char *strtok_r(char *s1, const char *s2, char **lasts);

#define MAX_DATA_LEN 128
#define EMPLOYEE_DELIMI TER_STR ":"
#define ATTR_DELIMITER_ STR ","
#define ATTR_DELIMITER_ CHAR ','
struct EmployeeInfo
{
char _name[MAX_DATA_LEN];
size_t _attributeCount ;
size_t *_attribute;

EmployeeInfo(co nst char *str):_attribut eCount(0), _attribute(NULL )
{
if(!strstr(str, ATTR_DELIMITER_ STR))
strcpy(_name, str);
else
{
char *temp1 = new char[strlen(str) + 1];
strcpy(temp1, str);
char *temp2 = temp1;

char* holdingBuf[1];

strcpy(_name, strtok_r(temp1, (const char *) ATTR_DELIMITER_ STR,
holdingBuf));
temp1 += (strlen(temp1) + 1);
_attributeCount = numChars(temp1, ATTR_DELIMITER_ CHAR) + 1;
_attribute = new size_t[_attributeCount];
memset(_attribu te, '\0', _attributeCount *sizeof(*_attri bute));

char* stringWithToken s = temp1;
char* token;
int i;
for(i =0;(token = strtok_r(string WithTokens, (const char *)
ATTR_DELIMITER_ STR, holdingBuf)) != NULL;
i++)
{
stringWithToken s = NULL;
_attribute[i] = atoi(token);
}

_attributeCount = i;

delete [] temp2;
}
}

virtual ~EmployeeInfo()
{
if(_attributeCo unt) delete [] _attribute;
}

void toString()
{
cout << "\nEmployee Name: " << _name << endl;
for(size_t i = 0; i < _attributeCount ; i++)
cout << "Attribute " << i << ": " << _attribute[i] << endl;
}

protected:
static size_t numChars(const char* str, const char delimiter)
{
size_t retVal = 0;
char ch;
for(size_t i = 0; ch = str[i]; i++)
{
if(ch == delimiter)
retVal++;
}
return retVal;
}
};
void
parseNameString (const char* nameString)
{
char *temp1 = new char[strlen(nameStri ng) + 1];
strcpy(temp1, nameString);
char *temp2 = temp1;
char* holdingBuf[1];

list<EmployeeIn fo*> empInfoList;

char* stringWithToken s = temp1;
char* token;
while((token = strtok_r(string WithTokens, (const char *)
EMPLOYEE_DELIMI TER_STR, holdingBuf)) != NULL)
{
stringWithToken s = NULL;
EmployeeInfo *newInfo = new EmployeeInfo(to ken);
empInfoList.pus h_back(newInfo) ;
}

delete [] temp2;

list<EmployeeIn fo*>::iterator itor, endOfList=empIn foList.end();

for(itor = empInfoList.beg in(); itor != endOfList; itor++)
{
EmployeeInfo* thisInfo = *itor;
thisInfo->toString();
}

for(itor = empInfoList.beg in(); itor != endOfList; itor++)
{

EmployeeInfo* thisInfo = *itor;
delete thisInfo;
}
}

int
main(int argc, char* argv[])
{
if(argc < 2)
{
cerr << "usage: " << argv[0] << " <inputfile>. Exiting ...." <<
endl;
exit(-1);
}

ifstream inFile(argv[1]);
if(!inFile)
{
cerr << "Error reading file " << argv[1] << " Exiting ...." <<
endl;
exit(-2);
}

char inputStr[MAX_DATA_LEN + 1];

/*
while(inFile >> inputStr)
{
cout << "** Parsing string " << inputStr << " **\n\n";
parseNameString (inputStr);
cout << "\n\n\n";
}
*/

while(inFile.ge tline(inputStr, MAX_DATA_LEN))
{
cout << "** Parsing string " << inputStr << " **\n\n";
parseNameString (inputStr);
cout << "\n\n\n";
}

exit(0);
}

[kbhat]

The posting of the entire source listing to the comp.lang.c newsgroup
was an inadvertent mistake which is deeply regretted.

[Jonathan Burd]

kb***@kaxy.com wrote:

I had written some code using strtok_r for parsing strings with two
"levels" of delimiters. The code was working like a charm, until it
suddenly broke one day. You see, I was applying strtok on the input
string itself, and as long as a variable string was passed to the
function, everything was hunky dory. Then one day somebody decided to
pass a constant string to my code ---- and everything came collapsing
down like a domino.

I believe that this is a pitfall into which many programmers may fall.
I have now changed the signature of my function to treat the input
string as a constant string, and I now making a local copy of the
string and operating on that. Of course, I now have to ensure that I
free up the dynamically allocated memory.
Ciao
KB

[I have posted my source listing to comp.sources.d]

char* strtok (char*, const char*);

strtok works by inserting '\0' in place of the delimiter each
successive call. Passing a string literal will definitely
cause trouble. Pass a string buffer. ;)

In fact, Herb-the-infamous-clown's book does what you did too!
Stay away from his books, unless you want an exercise
in debugging. ;)

Regards,
Jonathan.

--
"Women should come with documentation." - Dave

[Mike Wahler]

"Jonathan Burd" <jo***********@ REMOVEMEgmail.c om> wrote in message
news:35******** *****@individua l.net...

strtok works by inserting '\0' in place of the delimiter each
successive call. Passing a string literal will definitely
cause trouble. Pass a string buffer. ;)

In fact, Herb-the-infamous-clown's book does what you did too!
Wow, I knew he was spreading much misinformation, but
I didn't think it was *that* bad.

Stay away from his books, unless you want an exercise
in debugging. ;)

Too true.

-Mike

[Jonathan Burd]

Mike Wahler wrote:

"Jonathan Burd" <jo***********@ REMOVEMEgmail.c om> wrote in message
news:35******** *****@individua l.net...

strtok works by inserting '\0' in place of the delimiter each
successive call. Passing a string literal will definitely
cause trouble. Pass a string buffer. ;)

In fact, Herb-the-infamous-clown's book does what you did too!

Wow, I knew he was spreading much misinformation, but
I didn't think it was *that* bad.

<snip>

I wonder if he even compiled the example he has given.
I've put large crosses in that book wherever I've discovered
such errors. (That's the only book that I've marked so far.
Heh.)

Regards,
Jonathan.

--
"Women should come with documentation." - Dave

[Mike Wahler]

"Jonathan Burd" <jo***********@ REMOVEMEgmail.c om> wrote in message
news:35******** *****@individua l.net...

Mike Wahler wrote:

"Jonathan Burd" <jo***********@ REMOVEMEgmail.c om> wrote in message
news:35******** *****@individua l.net...

strtok works by inserting '\0' in place of the delimiter each
successive call. Passing a string literal will definitely
cause trouble. Pass a string buffer. ;)

In fact, Herb-the-infamous-clown's book does what you did too!

Wow, I knew he was spreading much misinformation, but
I didn't think it was *that* bad.

<snip>

I wonder if he even compiled the example he has given.
I've put large crosses in that book wherever I've discovered
such errors. (That's the only book that I've marked so far.
Heh.)

Depending upon the implementation( s) used, compiling and
testing might not have caught it. Especially on older
systems I've used, I've seen them happily accept modifying
literals, and 'seem to work'. I think stuff like this
(depending upon behavior of particular implementation( s)
rather than the language rules for determining correctness)
could easily be the cause of many of the errors in his work.

-Mike

[nimmi_srivastav]

In the interest of "public disclosure" I am reproducing the coding
sample that I see in my copy of "C: The Complete Reference"

--NS

#include "stdio.h" /* NS: Shouldn't this be <stdio.h>? */
#include "string.h" /* NS: Shouldn't this be <string.h>? */

void main(void)
{
char *p;

p = strtok("The summer soldier, the sunshine patriot", " ");
/* NS: Passing a constant string to strtok */

printf(p); /* NS: Isn't %s missing here? */
do {
p = strtok('\0', ", ");
if(p) printf("|%s", p);
} while(p);
}

[Jack Klein]

On 24 Jan 2005 18:37:09 -0800, ni************* @yahoo.com wrote in
comp.lang.c:

In the interest of "public disclosure" I am reproducing the coding
sample that I see in my copy of "C: The Complete Reference"

--NS

#include "stdio.h" /* NS: Shouldn't this be <stdio.h>? */
#include "string.h" /* NS: Shouldn't this be <string.h>? */
Yes, they both should. The #include "sometext" format is for
including source files, the #include <sometext> format is for
including standard headers, which need not be files.
void main(void)
This of course is just plain undefined behavior.
{
char *p;

p = strtok("The summer soldier, the sunshine patriot", " ");
/* NS: Passing a constant string to strtok */
No, your comment is in correct. He is passing the address of a string
literal, but the type of a string literal in C is "array of char" and
NOT "array of const char". Attempting to modify a string literal in C
does indeed produce undefined behavior, but not because the characters
are const, just because the C standard specifically says so.

Because attempting to modify a string literal is undefined, compilers
are free to actually make them const, but are not required to do so.
No strictly conforming program can tell one way or the other.
printf(p); /* NS: Isn't %s missing here? */
Not necessarily. If you pass a pointer to char to printf, assuming
that the pointer points to a valid string, printf will just output the
string. Unless of course the string happens to contain a conversion
specifier. If p points to "%s" for example, yet another incident of
undefined behavior occurs.
do {
p = strtok('\0', ", ");
if(p) printf("|%s", p);
} while(p);
}

Let it never be said that Schildt lacks the talent to cram multiple
examples of undefined behavior in such a small piece of code.

--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://www.eskimo.com/~scs/C-faq/top.html
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.l earn.c-c++
http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

[Jonathan Burd]

ni************* @yahoo.com wrote:

In the interest of "public disclosure" I am reproducing the coding
sample that I see in my copy of "C: The Complete Reference"

--NS

#include "stdio.h" /* NS: Shouldn't this be <stdio.h>? */
#include "string.h" /* NS: Shouldn't this be <string.h>? */

void main(void)
{
char *p;

p = strtok("The summer soldier, the sunshine patriot", " ");
/* NS: Passing a constant string to strtok */

printf(p); /* NS: Isn't %s missing here? */
No.

I prefer using puts() for testing certain string functions
instead of printf() though. Take, for example, a function
that generates a string consisting of printable characters when you
specify a range, say "^A-Z" (all printable characters except those in
the range A-Z; ^ negates), the string will contain '%' and if you use
printf to test such a routine, it will result in UB. It is
easy to forget that your string may contain '%', so when
testing string functions, generally avoid using printf() unless
you really need it and know what you are doing.
do {
p = strtok('\0', ", ");
if(p) printf("|%s", p);
} while(p);
}

Another error is his incorrect usage of feof().

See his usage in his book and then read this:
http://www.drpaulcarter.com/cs/common-c-errors.php#4.2

Regards,
Jonathan.

--
"Women should come with documentation." - Dave