String copy with pointers not working as expected

[kb]

Hi all,

I'm having trouble with the following code, of which I would like to
be able to get working without the use of string functions such as
strtok() and strcpy(), etc.. Please disregard off-topic nature of the
code where not pure C, as i believe the problem lies therein.

-------8<------

#include <stdio.h>
#include <stdlib.h>

#define MAXBUF 256

int main(void)
{
FILE *f;
char *d="cfg";
char *p;
char *q;
int x;

if((p=malloc(MAXBUF)) == NULL)
perror("malloc");

if((q=malloc(MAXBUF)) == NULL)
perror("malloc");

if((f=fopen(d,"r+")) < 0)
{
perror("fopen");
exit(1);
};

while(fgets(p,MAXBUF,f) != NULL)
{
while(*p != '\0')
{
if((*q++ = *p++) == '=')
{
printf("%s",q);
}
}
}
return 0;
}

------->8--------

I am trying to parse a flat file of the following form:

#conf file
rootdir=/test
opt1=foo
opt2=bar

And while it's easy to tokenize from the '=' and print the values
after that token, I am having a hard time consuming the chars before
the '='. Ouput is trash mostly, and i'm almost certain it's my use of
the 'q' pointer... where i doubt it points to what it should point to.

I've tried all manner of combination's of post/prefix to gobble up the
first part before the '=', but to no avail.

Any help is appreciated -

cheers

kB.

 

[Branimir Maksimovic]

Hi all,


I am trying to parse a flat file of the following form:

#conf file
rootdir=/test
opt1=foo
opt2=bar

And while it's easy to tokenize from the '=' and print the values
after that token, I am having a hard time consuming the chars before
the '='. Ouput is trash mostly, and i'm almost certain it's my use of
the 'q' pointer... where i doubt it points to what it should point to.

I've tried all manner of combination's of post/prefix to gobble up the
first part before the '=', but to no avail.

Any help is appreciated -

cheers

kB.

Here is another variant of your program

bmaxa@maxa:~$ gcc -Wall -O2 parse.c -o parse
parse.c: In function ‘main’:
parse.c:12: warning: unused variable ‘x’
bmaxa@maxa:~$ cat > cfg
#conf file
rootdir=/test
opt1=foo
opt2=bar
bmaxa@maxa:~$ ./parse
/test
foo
bar
bmaxa@maxa:~$ cat parse.c
#include <stdio.h>
#include <stdlib.h>

#define MAXBUF 256

int main(void)
{
FILE *f;
char *d="cfg";
char *p;
char *q;
int x;

if((p=malloc(MAXBUF)) == NULL)
perror("malloc");

if((f=fopen(d,"r+")) < 0)
{
perror("fopen");
exit(1);
};

while(fgets(p,MAXBUF,f) != NULL)
{
q = p;
while(*q != '\0')
{
if(*q++ == '=')
{
printf("%s",q);
}
}
}
return 0;
}

 

[Mark Bluemel]

Hi all,

I'm having trouble with the following code, of which I would like to
be able to get working without the use of string functions such as
strtok() and strcpy(), etc.. Please disregard off-topic nature of the
code where not pure C, as i believe the problem lies therein.

-------8<------

#include<stdio.h>
#include<stdlib.h>

#define MAXBUF 256

int main(void)
{
FILE *f;
char *d="cfg";
char *p;
char *q;
int x;

if((p=malloc(MAXBUF)) == NULL)
perror("malloc");

if((q=malloc(MAXBUF)) == NULL)
perror("malloc");

if((f=fopen(d,"r+"))< 0)
{
perror("fopen");
exit(1);
};

while(fgets(p,MAXBUF,f) != NULL)

Where does "p" point when you read the second record?

{
while(*p != '\0')
{
if((*q++ = *p++) == '=')
{
printf("%s",q);

Where does "q" point by this point?
And how is the string terminated?

}
}
}
return 0;
}

------->8--------

I am trying to parse a flat file of the following form:

#conf file
rootdir=/test
opt1=foo
opt2=bar

And while it's easy to tokenize from the '=' and print the values
after that token, I am having a hard time consuming the chars before
the '='. Ouput is trash mostly, and i'm almost certain it's my use of
the 'q' pointer... where i doubt it points to what it should point to.

I've tried all manner of combination's of post/prefix to gobble up the
first part before the '=', but to no avail.

Any help is appreciated -

Use different pointers for your buffers and the character manipulations,
for a start...

 

[Morris Keesan]

#include <stdio.h>
#include <stdlib.h>

#define MAXBUF 256

int main(void)
{
FILE *f;
char *d="cfg";
char *p;
char *q;
int x;

if((p=malloc(MAXBUF)) == NULL)
perror("malloc");

if((q=malloc(MAXBUF)) == NULL)
perror("malloc");

fopen returns a NULL pointer on error. Testing a pointer for
a negative value is not particularly useful.

if((f=fopen(d,"r+")) < 0)
{
perror("fopen");
exit(1);
};

The semicolon after the ending brace is unnecessary. In this case,
all you've done is to insert an empty statement, but this is a dangerous
habit to get into.

while(fgets(p,MAXBUF,f) != NULL)

As has been pointed out, the second time you get here, p and q
no longer point to their allocated blocks.

{
while(*p != '\0')
{
if((*q++ = *p++) == '=')
{

And here, the first time, q points to uninitialized data.
Each time you execute "*q++ = *p++", you copy one character, and
move p to the next unexamined character, and move q to the next
character which hasn't had anything copied into it.

printf("%s",q);
}
}
}
return 0;
}

------->8--------

I am trying to parse a flat file of the following form:

#conf file
rootdir=/test
opt1=foo
opt2=bar

And while it's easy to tokenize from the '=' and print the values
after that token, I am having a hard time consuming the chars before
the '='. Ouput is trash mostly, and i'm almost certain it's my use of
the 'q' pointer... where i doubt it points to what it should point to.

I've tried all manner of combination's of post/prefix to gobble up the
first part before the '=', but to no avail.

I don't see why you're using malloc for a buffer of fixed size which
gets allocated just once. A fairly simple way to do this would be
(WARNING: UNTESTED CODE)

char buf[MAXBUF];
char *q;

/* .. open the file .. */

while (fgets(buf, sizeof buf, f) != NULL)
{
for (q = buf; *q++ != '='; ) {}
fputs(q, stdout);
}

Note that this prints the values on the right-hand sides of the '='
characters, while discarding the keys on the left-hand sides.

Frankly, C is not the language I would use for doing this, unless this
parsing were part of a larger program that I had chosen C for.

 

[Keith Thompson]

Something I don't think anyone else has pointed out: if malloc() returns
NULL, the program prints an error message *and then keeps running as if
nothing had gone wrong*. exit(EXIT_FAILURE)

fopen returns a NULL pointer on error. Testing a pointer for
a negative value is not particularly useful.

Nor is it particularly legal; this should have produced a warning or
fatal error message, since it violates a constraint.

If you didn't get a warning on the comparison, you need to find out
how to make your compiler warn about this kind of thing.

(Possibly the compiler treated 0 as a null pointer constant, which it
is, but it's of type int and there's no implicit int-to-pointer
conversion in this context.)

The semicolon after the ending brace is unnecessary. In this case,
all you've done is to insert an empty statement, but this is a dangerous
habit to get into.

And 1 is not a portable value to pass to exit(); use EXIT_FAILURE.

[...]

 

[kb]

Thank you all for your posts - it's given me better insight as to
where I am going wrong.

The new code you wrote is much better and clean. I now have to figure
out how to consume everything /up to/ the '=', not after, as this is a
smaller function of a larger program which I am redesigning, where I
will be parsing a config file (and expecting/dealing with errors in
the config file, etc..).


Cheers

kB

 

[David Thompson]

kb wrote:

q = p;
while (fgets(q, MAXBUF, fp) != NULL) {
while (*q != '\0' && *q != '=') {
++q;
}
while (*q == '=') {
++q;
}

Instead of skipping exactly one = as your prior code did, this skips
possibly multiple =, but the OP wanted to skip none.

printf(q);

This optimization is unsafe, if the input file might (accidentally or
maliciously) contain any % on a right-hand-part. Your prior
printf("%s",q) is safe, or fputs(q,stdout) is a valid optimization.
(To be exact, it's definitely a valid change, and _probably_ an
optimization but that _could_ be implementation dependent.)

q = p;
}

Nit: I dislike duplication like this. It requires the reader (often
the coder a year or 5 later) to re-think whether the two things are
supposed to be the same or not. Especially if the code is a little
more complicated than this case. I would do:

while( fgets (p, MAXBUF, fp) != NULL ){
q = p;
/* parse using q as before */
}

I might even move the declaration locally:
while( fgets (p, MAXBUF, fp) != NULL ){
/*could be const*/ char * q = p;
....
}