strtok problem

[Jase Schick]

Hi all,
I have written the following code:

/* strtok example */
#include <stdio.h>
#include <string.h>

static const char * const resultFileName = "param.txt";

int main ()
{

FILE *fPtr;
char str[300];
char * pch;
int len;
char savedstr[300];

fPtr =fopen(resultFileName, "r");

if(fPtr == NULL)
{
printf("File doesnot exist");
return 1;
}

else
{
while(!feof(fPtr))
{
fgets(str,288,fPtr);
len = strlen(str);
str[len-1]=0;
for ( (pch=strtok(str,","));(pch = strtok(NULL,","));(pch
!=NULL))
{
strncpy(savedstr,pch,len);
printf("Saved String =%s\n",savedstr);
}

}

}

fclose(fPtr);
return 0;

}

I have a text file like the following:
param.txt:

"ram","lax","deepak"
10,20,30
40,80,100
70,90,100
and so on...

When I use strtok(delim ,) the problem is that the string "ram" never
gets stored in savedstr array while from "lax" onwards it gets stored.

1. Could any body suggest a better method so that I can store "ram"
also in the saved string and tokenize based on comma separated
delimiter?

2. Is there anything in C like (map in C++) so that I can store:

(key)
"ram"---->10,40,70(In an array of integer values)
"lax"---->20,80,90

Could anybody help me in this regard?

Best:

Jase

--
For God so loved the world, that He gave His only begotten Son,
that whoever believes in Him shall not perish, but have eternal life. For
God did not send the Son into the world to judge the world, but that the
world might be saved through Him. He who believes in Him is not judged;
he who does not believe has been judged already, because he has not
believed in the name of the only begotten Son of God.
( John 3:16 )

 

[John Bode]

Hi all,
       I have written the following code:

/* strtok example */ [snip]

       while(!feof(fPtr))
       {
          fgets(str,288,fPtr);

You could also write

fgets(str, sizeof str, fPtr);

That way you don't have to worry about magic numbers.

          len = strlen(str);
          str[len-1]=0;
          for ( (pch=strtok(str,","));(pch = strtok(NULL,","));(pch
!=NULL))

You have your order of operations mixed up; the test for null should
be the second clause, while the call to strtok() should be the third
clause:

for (pch = strtok(str, ","); pch != NULL; pch =
strtok(NULL, ","))

The second clause is the test condition, while the third clause is the
update expression. Your code "worked" for the rest of the file
because the test condition updated pch.

[snip]

2. Is there anything in C like (map in C++) so that I can store:

(key)
"ram"---->10,40,70(In an array of integer values)
"lax"---->20,80,90

Not as part of the standard library; you'll have to either build your
own data structure to do this (not that hard, but I understand not
wanting to do it) or find a third-party library that does it for you.

 

[John Bode]

       I have written the following code:

/* strtok example */
[snip]

       while(!feof(fPtr))
       {

Gah! Just saw Richard's reply, and he pointed out something I
missed. DO NOT use feof() as your loop condition, because you'll wind
up executing the loop once too often. feof() doesn't return true
until *after* you attempt to read past the end of the file.

The better method is to test the result of the input call, such as:

while (fgets(str, sizeof str, fptr) != NULL)
{
/* process str */
}

/* *NOW* check for end of file */

if (feof(fptr))
{
/* Hit end of file, shut down normally */
}
else
{
/* Encountered an error on input, handle as appropriate */
}

 

[bart.c]

I have a text file like the following:
param.txt:

"ram","lax","deepak"
10,20,30
40,80,100
70,90,100
and so on...

This is not clear; does the file consist of (where x is a string, and n is a
number):

x,x,x
n,n,n
n,n,n
n,n,n
x,x,x
n,n,n
n,n,n
n,n,n
etc

or:

x,x,x
n,n,n
n,n,n
.....
n,n,n

or:

x,x,x
n,n,n
....
x,x,x
n,n,n,
....
x,x,x
etc

(in this last case, determining the start of the next x,x,x line is tricky).

2. Is there anything in C like (map in C++) so that I can store:

(key)
"ram"---->10,40,70(In an array of integer values)
"lax"---->20,80,90

No. You have to deal with all the details yourself. But if there are only
ever 3 numbers in each array, this will simplify things, depending what you
intend to do about searching and sorting the different entries.

(But in this case, it's not clear what it is about the 3 strings that sets
them aside from other sets in the file.)

 

[Seebs]

while(!feof(fPtr))
{
fgets(str,288,fPtr);

Other people have commented on various bits of this, but I'm just curious:

Where did you pick up this idiom? It's not the way people normally write
loops in C (because it doesn't work).

-s

 

[Denis McMahon]

while(!feof(fPtr))
{
fgets(str,288,fPtr);
len = strlen(str);
str[len-1]=0;
for ( (pch=strtok(str,","));(pch = strtok(NULL,","));(pch
!=NULL))
{
strncpy(savedstr,pch,len);
printf("Saved String =%s\n",savedstr);
}

}

with:

while(!feof(fPtr)) {
fgets(str,288,fPtr);
len = strlen(str);
if (len > 5) {
str[len-1]=0;
pch=strtok(str,",");
do {
strncpy(savedstr,pch,len);
printf("Saved String =%s\n",savedstr);
pch = strtok(NULL,",");
} while (pch != NULL);
}
}

the (len > 5) test is to ensure that a string has a minimum of "data
delim data delim data newline".

I can't remember why this is better, but I seem to recall that for
strtok doing things this way round with "do {} until ();" seems to work
better.

You could also set the delimiter to ",\"\n\r" which would remove the
need to overwrite str[strlen(str)-1] with 0;

Rgds

Denis McMahon

 

[Jase Schick]

while(!feof(fPtr))
{
fgets(str,288,fPtr);
len = strlen(str);
str[len-1]=0;
for ( (pch=strtok(str,","));(pch = strtok(NULL,","));(pch
!=NULL))
{
strncpy(savedstr,pch,len);
printf("Saved String =%s\n",savedstr);
}

}

with:

while(!feof(fPtr)) {
fgets(str,288,fPtr);
len = strlen(str);
if (len > 5) {
str[len-1]=0;
pch=strtok(str,",");
do {
strncpy(savedstr,pch,len);
printf("Saved String =%s\n",savedstr); pch = strtok(NULL,",");
} while (pch != NULL);
}
}

the (len > 5) test is to ensure that a string has a minimum of "data
delim data delim data newline".

I can't remember why this is better, but I seem to recall that for
strtok doing things this way round with "do {} until ();" seems to work
better.

You could also set the delimiter to ",\"\n\r" which would remove the
need to overwrite str[strlen(str)-1] with 0;

Rgds

Denis McMahon

Denis

Thanks.

Many of the other responses erroneously removed the EOF check that I need
to keep!

Best:

Jase

--
For God so loved the world, that He gave His only begotten Son,
that whoever believes in Him shall not perish, but have eternal life. For
God did not send the Son into the world to judge the world, but that the
world might be saved through Him. He who believes in Him is not judged;
he who does not believe has been judged already, because he has not
believed in the name of the only begotten Son of God.
( John 3:16 )

 

[Denis McMahon]

Many of the other responses erroneously removed the EOF check that I need
to keep!

I didn't like that when I re-read it, although I had checked that it
worked.

You need to do some sort of check after the fgets to validate that
what's been read in looks reasonable, checking the length is very crude
and will probably bite you, but it's better than nothing.

Could you have blank lines in the file?
Is the file always terminated with a blank line, or a data line, or
could it be either.

You may also want to check the return status of fgets first, maybe:

char *status;

while (!feof(fPtr)) {
status = fgets(str,288,fPtr);
if (status == NULL) break;
len = strlen(str);
etc .....
}

Rgds

Denis McMahon

 

[John Bode]

I didn't like that when I re-read it, although I had checked that it
worked.

You need to do some sort of check after the fgets to validate that
what's been read in looks reasonable, checking the length is very crude
and will probably bite you, but it's better than nothing.

Could you have blank lines in the file?
Is the file always terminated with a blank line, or a data line, or
could it be either.

You may also want to check the return status of fgets first, maybe:

char *status;

while (!feof(fPtr)) {
status = fgets(str,288,fPtr);
if (status == NULL) break;
len = strlen(str);
etc .....

}

Rgds

Denis McMahon

Again, feof() won't return true until *after* you attempt to read past
the end of file, which is why I removed it as the loop condition.
Check the result of fgets() first; if it's NULL, *then* check for EOF
using feof(). Another possible restructuring is:

int done = 0;
...
while (!done)
{
if (fgets(str, sizeof str, fptr) != NULL)
{
/* process as normal */
}
else
{
if (feof(fptr))
{
/* Hit EOF, terminate normally */
done = 1;
}
else
{
/* Error on input; handle as appropriate */
}
}
}

 

[Seebs]

Many of the other responses erroneously removed the EOF check that I need
to keep!

Your EOF check is incorrect, and does not work; in particular, it would
typically cause the last time to be processed a second time -- but,
conveniently, strtok() will already have mangled the data buffer.

-s

 

[Nick Keighley]

Denis McMahon writes:

while(!feof(fPtr)) {
  fgets(str,288,fPtr); [...]
}

Denis McMahon

it's customery to snip signatures

Denis

Thanks.

Many of the other responses erroneously removed the EOF check that I need
to keep!

you don't becasue it's wrong. Where did you pick up this way of
reading files from? (we're curious because there's a well known book
that teaches it...)

 

[Nick Keighley]

On 08/06/10 22:20, Jase Schick wrote:

I didn't like that when I re-read it, although I had checked that it
worked.

You need to do some sort of check after the fgets to validate that
what's been read in looks reasonable, checking the length is very crude
and will probably bite you, but it's better than nothing.

Could you have blank lines in the file?
Is the file always terminated with a blank line, or a data line, or
could it be either.

You may also want to check the return status of fgets first, maybe:

char *status;

while (!feof(fPtr)) {
status = fgets(str,288,fPtr);
if (status == NULL) break;
len = strlen(str);
etc .....

}

from the comp.lang.c FAQ
FAQ 12.2 "Why does the simple line-copying loop while(!feof(infp))
{ fgets(buf, MAXLINE, infp); fputs(buf, outfp); } copy the last line
twice?"

http://c-faq.com/stdio/feof.html

the usual advice is to read the rest of the section as well. And then
read the whole FAQ.

 

[Jase Schick]

No, he definitely wants to check the return status of fgets, but...



...not like that!

Richard

I don't see anything wrong with this. Here's the documentation:

gets() and fgets() return s on success, and NULL on error or when end of
file occurs while no characters have been read.

So checking against NULL is correct, and then an additional check is
needed to distinguish end of file from an I/O error.

Best:

Jase

--
For God so loved the world, that He gave His only begotten Son,
that whoever believes in Him shall not perish, but have eternal life. For
God did not send the Son into the world to judge the world, but that the
world might be saved through Him. He who believes in Him is not judged;
he who does not believe has been judged already, because he has not
believed in the name of the only begotten Son of God.
( John 3:16 )

 

[John Bode]

Richard

I don't see anything wrong with this. Here's the documentation:

gets() and fgets() return s on success, and NULL on error or when end of
file occurs while no characters have been read.

So checking against NULL is correct, and then an additional check is
needed to distinguish end of file from an I/O error.

Right, but don't make the result of feof() be your *loop condition*;
that's the point Richard's trying to make. Any loop structured like

while (!feof(fp)) {...}

will execute once too often, since feof() won't return true until
*after* you attempt to read past the end of the file. If you have ten
bytes in your file, and you read one byte at a time, reading the tenth
byte won't cause feof() to return true, even though there are no bytes
left before EOF. You have to try *and fail* to read the next byte
before feof() returns true.

So the suggestion is to use the result of fgets() as your loop
condition:

while (fgets(str, sizeof str, fp) != NULL)
{
...
}
if feof(fp) { /* hit end of file, exit normally */ }
else { /* read failed on some other error, handle as appropriate
*/ }

or, if you want to try to recover from read errors (how, I wouldn't
know), use a flag:

int done = 0;
...
while (!done)
{
if (!fgets(str, sizeof str, fp))
{
if (feof(fp))
done = 1;
else
/* try to recover from the error somehow */
}
else
/* process str */
}

The point is, you check the result of fgets() first and if it's NULL,
*then* you check for EOF, not the other way around.

 

[Ben Bacarisse]

Right, but don't make the result of feof() be your *loop condition*;
that's the point Richard's trying to make.

So the suggestion is to use the result of fgets() as your loop
condition:

while (fgets(str, sizeof str, fp) != NULL)
{
...
}
if feof(fp) { /* hit end of file, exit normally */ }
else { /* read failed on some other error, handle as appropriate
*/ }

Strong ack (with ()s round the if condition). There are a lot of overly
complex (or wrong) input loops out there.

or, if you want to try to recover from read errors (how, I wouldn't
know), use a flag:

int done = 0;
...
while (!done)
{
if (!fgets(str, sizeof str, fp))
{
if (feof(fp))
done = 1;
else
/* try to recover from the error somehow */
}
else
/* process str */
}

I would not use a flag here (well, in a way I would, but it would not be
my flag). I'd probably write:

do {
while (fgets(str, sizeof str, fp)) {
/* process str */
}
if (ferror(fp)) {
/* correct the error if you can */
}
} while (ferror(fp));

which I think makes the intent clear. I don't mind the two ferror
tests -- I think it is simpler than testing !done, fgets() and feof(fp)
to get the same effect. YMMV.

 

[Seebs]

I don't see anything wrong with this.

That is why people have been warning you.

Your initial loop was just:
while (!feof(fPtr)) {
fgets(str, 288, fPtr);
/* use str */
}

This was definitely wrong. And my question about it still stands; where
did you get this idiom? It's not a standard C loop.

gets() and fgets() return s on success, and NULL on error or when end of
file occurs while no characters have been read.
Right.

So checking against NULL is correct,
Right.

and then an additional check is
needed to distinguish end of file from an I/O error.

Let's think this through.

Imagine that an I/O error has occurred. What will your loop do?

It will loop, forever, returning NULL from fgets because it can't read
anything due to the I/O error. It will never terminate.

That's why people usually loop on fgets(). It is extremely rare that you
need to know *why* there's nothing more to read, but it is crucial to know
*whether* there's nothing more to read.

-s

 

[Phil Carmody]

Hi all,
I have written the following code:

/* strtok example */
#include <stdio.h>
#include <string.h>

static const char * const resultFileName = "param.txt";

int main ()
{

FILE *fPtr;
char str[300];
char * pch;
int len;
char savedstr[300];

fPtr =fopen(resultFileName, "r");

if(fPtr == NULL)
{
printf("File doesnot exist");
return 1;
}

else
{
while(!feof(fPtr))
{
fgets(str,288,fPtr);
len = strlen(str);
str[len-1]=0;

And if len==0?

Phil