String copy with pointers not working as expected

Discussion in 'C Programming' started by kb, Dec 15, 2010.

  1. kb

    kb Guest

    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.
    kb, Dec 15, 2010
    #1
    1. Advertising

  2. On 12/15/2010 04:23 PM, kb wrote:
    > 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;
    }
    Branimir Maksimovic, Dec 15, 2010
    #2
    1. Advertising

  3. kb

    Mark Bluemel Guest

    On 12/15/2010 03:23 PM, kb wrote:
    > 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...
    Mark Bluemel, Dec 15, 2010
    #3
  4. On Wed, 15 Dec 2010 10:23:10 -0500, kb <> wrote:

    > #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.

    --
    Morris Keesan --
    Morris Keesan, Dec 15, 2010
    #4
  5. "Morris Keesan" <> writes:
    > On Wed, 15 Dec 2010 10:23:10 -0500, kb <> wrote:
    >
    >> #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");


    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.


    >> if((f=fopen(d,"r+")) < 0)


    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.)

    >> {
    >> 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.


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

    [...]

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
    Keith Thompson, Dec 15, 2010
    #5
  6. kb

    kb Guest

    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
    kb, Dec 16, 2010
    #6
  7. On Thu, 16 Dec 2010 08:56:55 -0500, pete <>
    wrote:

    > kb wrote:


    > > 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, <snip>


    > 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;
    ....
    }
    David Thompson, Dec 27, 2010
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Richard Huff
    Replies:
    2
    Views:
    401
    Richard Huff
    Jan 6, 2004
  2. Natty Gur
    Replies:
    1
    Views:
    1,889
    Marshal Antony
    Mar 3, 2004
  3. Alex
    Replies:
    2
    Views:
    1,223
  4. Replies:
    26
    Views:
    2,113
    Roland Pibinger
    Sep 1, 2006
  5. cerr

    pointers, pointers, pointers...

    cerr, Apr 7, 2011, in forum: C Programming
    Replies:
    12
    Views:
    672
Loading...

Share This Page