Understanding some code

Discussion in 'C Programming' started by parjit, Aug 24, 2010.

  1. parjit

    parjit Guest

    Hi

    I'm working on a small c project. I need to read lines from stdin, but
    because gets is known to be an undefined function I've been given the
    code below to use instead. It works fine but I'd like to understand what
    its doing and I just find it really confusing.

    Can anyone explain to me what's going on here. Thanks.

    #include <string.h>
    char *safegets(char *buf, int sz)
    {
    char *r,*p;
    return (buf==NULL || sz<1) ? NULL :
    sz==1 ? *buf='\0', buf :
    !(r=fgets(buf,sz,stdin)) ? r :
    (p = strchr(buf, '\n')) ? *p=0, r : r;
    }
    parjit, Aug 24, 2010
    #1
    1. Advertising

  2. parjit

    Seebs Guest

    On 2010-08-24, parjit <> wrote:
    > I'm working on a small c project. I need to read lines from stdin, but
    > because gets is known to be an undefined function I've been given the
    > code below to use instead. It works fine but I'd like to understand what
    > its doing and I just find it really confusing.


    That is because this code is confusing.

    > Can anyone explain to me what's going on here. Thanks.


    > #include <string.h>
    > char *safegets(char *buf, int sz)
    > {
    > char *r,*p;
    > return (buf==NULL || sz<1) ? NULL :
    > sz==1 ? *buf='\0', buf :
    > !(r=fgets(buf,sz,stdin)) ? r :
    > (p = strchr(buf, '\n')) ? *p=0, r : r;
    > }


    Okay, first a top-level view: Someone who thinks he's a lot smarter than
    he actually is is trying to show off. I used to write code like this,
    maybe fifteen or twenty years ago.

    Okay, let's start by expanding this a bit:

    char *safegets(char *buf, int size)
    {
    char *temp1, *temp2;
    return (buf == NULL || size < 1) ? NULL :
    size == 1 ? (*buf = '\0', buf) :
    !(temp1 = fgets(buf, size, stdin)) ? temp1 :
    (temp2 = strchr(buf, '\n')) ? (*temp2 = 0, temp1) :
    temp1;
    }

    Wow, that's awful.

    Quick intro:
    return x ? y : z
    is basically the same as
    if (x) {
    return y;
    } else {
    return z;
    }

    ?: is like an if/then, except you use it as part of an expression,
    instead of on statements. So you could do something like
    abs_of_x = (x < 0) ? (-1 * x) : (x);
    and that's the same effect as
    if (x < 0) {
    abs_of_x = (-1 * x);
    } else {
    abs_of_x = x;
    }

    Let's convert that to if/else:
    char *safegets(char *buf, int size);
    {
    char *temp1, *temp2;
    if (buf == NULL || size < 1) {
    return NULL;
    } else if (size == 1) {
    *buf = '\0';
    return buf;
    } else if (!(temp1 = fgets(buf, size, stdin))) {
    return temp1;
    } else if (temp2 = strchr(buf, '\n')) {
    *temp2 = 0;
    return temp1;
    } else {
    return temp1;
    }
    }

    Okay, this is still crap. I would not approve this code in a tech
    review but it's getting legible. Lemme give it some comments:

    /* read at most size characters into buf, resulting in a
    * null-terminated string. If buf is null or size is not
    * at least 1, returns null. If fgets() fails, returns
    * null. Otherwise, returns buf, with the first newline
    * (if any) replaced with a null byte. Since fgets() in
    * theory stops with the newline, that just trims a trailing
    * newline.
    */
    char *safegets(char *buf, int size);
    {
    char *temp1, *temp2;
    if (buf == NULL || size < 1) {
    /* if buf is NULL, or we've said that less than
    * one byte is available, return a NULL pointer;
    * you give me invalid inputs, I give you invalid
    * outputs.
    */
    return NULL;
    } else if (size == 1) {
    /* if we have exactly one byte, it has to be
    * the null terminator.
    */
    *buf = '\0';
    return buf;
    } else if (!(temp1 = fgets(buf, size, stdin))) {
    /* assign the results of fgets() into a
    * new value named temp1. If it's "false"
    * (a null pointer), return that newly
    * generated null pointer.
    */
    return temp1;
    } else if (temp2 = strchr(buf, '\n')) {
    /* if we find a newline in the string, replace
    * it with a null byte. Return the pointer
    * returned by fgets. Which is identical to buf,
    * mind you.
    */
    *temp2 = 0;
    return temp1;
    } else {
    /* there was no newline, but we got a string,
    * return it unmodified.
    */
    return temp1;
    }
    }

    Ugh. Not very consistent, but at least it makes sense.

    char *safegets(char *buf, int size);
    {
    char *temp1, *temp2;
    if (buf == NULL || size < 1) {
    return NULL;
    } else if (size == 1) {
    *buf = '\0';
    return buf;
    }
    temp1 = fgets(buf, size, stdin);
    if (!temp1) {
    /* fgets failed */
    return NULL;
    }
    temp2 = strchr(buf, '\n');
    if (temp2) {
    /* found a newline, trim it */
    *temp2 = '\0';
    }
    return buf;
    }

    That's a little clearer. Still, it's pretty kludgy. Let's see
    if we can't make it a little friendlier. At this point, it's more
    clear what the goal is: The goal is to write a wrapper around fgets()
    which strips the trailing newline. For historical reasons, "gets()"
    trims the terminating newline, fgets() doesn't.

    Here's a first pass:

    char *safegets(char *buf, int size) {
    int c;
    int count = 1;
    char *s = buf;
    if (s && size > 0) {
    while ((c = getchar()) != EOF &&
    c != '\n' &&
    count++ < size) {
    *s++ = c;
    }
    *s++ = '\0';
    return buf;
    } else {
    /* invalid arguments */
    return NULL;
    }
    }

    Here's my test program:

    #include <stdio.h>

    char *safegets(char *buf, int size) {
    int c;
    int count = 1;
    char *s = buf;
    if (s && size > 0) {
    while ((c = getchar()) != EOF &&
    c != '\n' &&
    count++ < size) {
    *s++ = c;
    }
    *s++ = '\0';
    return buf;
    } else {
    /* invalid arguments */
    return NULL;
    }
    }

    int
    main(void) {
    char buf[10] = { "xxxxxxxxx" };
    int i;

    printf("doing safegets, length 6:\n");
    safegets(buf, 6);
    for (i = 0; i < 8; ++i) {
    printf("\t[%d]: 0x%02x [%c]\n",
    i,
    (unsigned char) buf,
    isprint((unsigned char) buf) ?
    buf : '.');
    }
    printf("[5] or earlier should be 0x00 [.], following should be [x].\n");
    return 0;
    }

    This passed the obvious edge cases:
    * values less than 5 characters long
    * exactly 5 characters plus a newline
    * newline with following characters
    * more than 6 characters

    -s
    --
    Copyright 2010, all wrongs reversed. Peter Seebach /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
    Seebs, Aug 24, 2010
    #2
    1. Advertising

  3. parjit

    Ian Collins Guest

    On 08/25/10 08:51 AM, parjit wrote:
    > Hi
    >
    > I'm working on a small c project. I need to read lines from stdin, but
    > because gets is known to be an undefined function I've been given the
    > code below to use instead. It works fine but I'd like to understand what
    > its doing and I just find it really confusing.
    >
    > Can anyone explain to me what's going on here. Thanks.
    >
    > #include<string.h>
    > char *safegets(char *buf, int sz)
    > {
    > char *r,*p;
    > return (buf==NULL || sz<1) ? NULL :
    > sz==1 ? *buf='\0', buf :
    > !(r=fgets(buf,sz,stdin)) ? r :
    > (p = strchr(buf, '\n')) ? *p=0, r : r;
    > }


    If the project contains code like this, run away!

    --
    Ian Collins
    Ian Collins, Aug 24, 2010
    #3
  4. parjit

    Paul N Guest

    On 24 Aug, 21:51, parjit <> wrote:
    > Hi
    >
    > I'm working on a small c project. I need to read lines from stdin, but
    > because gets is known to be an undefined function I've been given the
    > code below to use instead. It works fine but I'd like to understand what
    > its doing and I just find it really confusing.
    >
    > Can anyone explain to me what's going on here. Thanks.
    >
    > #include <string.h>
    > char *safegets(char *buf, int sz)
    > {
    >    char *r,*p;
    >    return (buf==NULL || sz<1) ? NULL :
    >      sz==1 ? *buf='\0', buf :
    >      !(r=fgets(buf,sz,stdin)) ? r :
    >      (p = strchr(buf, '\n')) ? *p=0, r : r;
    > }


    I think whoever wrote that code may be messing you about.

    If you say something like

    return a ? b : c;

    then this is the same as:

    if (a) return b; else return c;

    and because "return" causes the following statements to be missed out,
    this is the same as:

    if (a) return b;
    return c;

    So applying that a few times and making a few other small changes, we
    get:

    char *safegets(char *buf, int sz)
    {
    char *r,*p;
    if (buf==NULL || sz<1) return NULL;
    if (sz==1) { buf[0]='\0'; return buf; }
    r=fgets(buf,sz,stdin);
    if (!r) return r;
    p = strchr(buf, '\n');
    if (p) { p[0]=0; return r; }
    return r;
    }

    which in turn means:
    bail out if buf is NULL or sz (size) is too small
    return empty string if there's no space for anything longer
    read in one line, or sz-1 characters, whichever is shorter
    return NULL if that failed
    look for a newline
    replace it if you find one
    return the string read (same as buf)

    Hope that helps.
    Paul.
    Paul N, Aug 24, 2010
    #4
  5. parjit

    Uno Guest

    Seebs wrote:

    > Okay, first a top-level view: Someone who thinks he's a lot smarter than
    > he actually is is trying to show off. I used to write code like this,
    > maybe fifteen or twenty years ago.
    >
    > Okay, let's start by expanding this a bit:


    Seebs gives himself to be this person who understands the learning
    process, indeed can even divine what others are learning.

    And, here, he's answering somebody's homework. This is the first
    worksheet the prof hands out.

    Way to expand The Topic. How about if we call c.l.c. Seebs' blog now?
    --
    Uno
    Uno, Aug 25, 2010
    #5
  6. parjit

    John Kelly Guest

    On Tue, 24 Aug 2010 18:57:33 -0600, Uno <> wrote:

    >Seebs wrote:
    >
    >> Okay, first a top-level view: Someone who thinks he's a lot smarter than
    >> he actually is is trying to show off. I used to write code like this,
    >> maybe fifteen or twenty years ago.
    >>
    >> Okay, let's start by expanding this a bit:

    >
    >Seebs gives himself to be this person who understands the learning
    >process, indeed can even divine what others are learning.
    >
    >And, here, he's answering somebody's homework. This is the first
    >worksheet the prof hands out.
    >
    >Way to expand The Topic. How about if we call c.l.c. Seebs' blog now?


    There's no business like show business.



    --
    Web mail, POP3, and SMTP
    http://www.beewyz.com/freeaccounts.php
    John Kelly, Aug 25, 2010
    #6
  7. parjit

    Seebs Guest

    On Tue, 24 Aug 2010 18:57:33 -0600, Uno <> wrote:
    >And, here, he's answering somebody's homework. This is the first
    >worksheet the prof hands out.


    Is it? What university, or course? We could, of course verify this. I
    didn't see anything obviously marking it as homework, and I have a really
    hard time imagining that being the first of ANYTHING in a C course, because
    you generally get through a lot of other stuff before you start working
    with nested ?:.

    -s
    --
    Copyright 2010, all wrongs reversed. Peter Seebach /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
    Seebs, Aug 25, 2010
    #7
  8. parjit

    Eric Sosman Guest

    On 8/24/2010 9:36 PM, Seebs wrote:
    > On Tue, 24 Aug 2010 18:57:33 -0600, Uno<> wrote:
    >> And, here, he's answering somebody's homework. This is the first
    >> worksheet the prof hands out.

    >
    > Is it? What university, or course? We could, of course verify this. I
    > didn't see anything obviously marking it as homework, and I have a really
    > hard time imagining that being the first of ANYTHING in a C course, because
    > you generally get through a lot of other stuff before you start working
    > with nested ?:.


    ... and especially with nested ?: used stupidly. The code as
    shown won't compile, and even with the obvious fix it won't survive
    code review. It's on a par with `if((!isspace(ch)!=1) == 0)'.

    There are two audiences for source code: Compilers and programmers.
    The latter is by far the more important; cater to their needs before
    worrying about those of the former.

    --
    Eric Sosman
    lid
    Eric Sosman, Aug 25, 2010
    #8
  9. parjit

    Seebs Guest

    On 2010-08-25, Eric Sosman <> wrote:
    > ... and especially with nested ?: used stupidly. The code as
    > shown won't compile, and even with the obvious fix it won't survive
    > code review. It's on a par with `if((!isspace(ch)!=1) == 0)'.


    It's really bad. And the interesting thing is, despite Uno's confident
    assertion that this is the "first worksheet", I can find no instances
    of that code anywhere except this thread.

    But mostly, it's just plain too advanced to be a first handout, but
    it's very plausible as a cargo cult thing someone would hand you
    and tell you it's great. I will say, though. If you look at some
    of the code we see in this newsgroup, hypothesizing a teacher who
    would hand that out has substantial explanatory power.

    -s
    --
    Copyright 2010, all wrongs reversed. Peter Seebach /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
    Seebs, Aug 25, 2010
    #9
  10. parjit

    Uno Guest

    Seebs wrote:
    > On 2010-08-25, Eric Sosman <> wrote:
    >> ... and especially with nested ?: used stupidly. The code as
    >> shown won't compile, and even with the obvious fix it won't survive
    >> code review. It's on a par with `if((!isspace(ch)!=1) == 0)'.

    >
    > It's really bad. And the interesting thing is, despite Uno's confident
    > assertion that this is the "first worksheet", I can find no instances
    > of that code anywhere except this thread.
    >
    > But mostly, it's just plain too advanced to be a first handout, but
    > it's very plausible as a cargo cult thing someone would hand you
    > and tell you it's great. I will say, though. If you look at some
    > of the code we see in this newsgroup, hypothesizing a teacher who
    > would hand that out has substantial explanatory power.


    But you certainly don't think this occured in production code, and this
    is the first week of class for people taking computer programming in a
    lot of places.

    If that place is India, this might be high school stuff.
    --
    Uno
    Uno, Aug 25, 2010
    #10
  11. parjit

    Willem Guest

    Seebs wrote:
    ) <big snip>
    ) Ugh. Not very consistent, but at least it makes sense.
    )
    ) char *safegets(char *buf, int size);
    ) {
    ) char *temp1, *temp2;
    ) if (buf == NULL || size < 1) {
    ) return NULL;
    ) } else if (size == 1) {
    ) *buf = '\0';
    ) return buf;
    ) }
    ) temp1 = fgets(buf, size, stdin);
    ) if (!temp1) {
    ) /* fgets failed */
    ) return NULL;
    ) }
    ) temp2 = strchr(buf, '\n');
    ) if (temp2) {
    ) /* found a newline, trim it */
    ) *temp2 = '\0';
    ) }
    ) return buf;
    ) }
    )
    ) That's a little clearer. Still, it's pretty kludgy. Let's see
    ) if we can't make it a little friendlier. At this point, it's more
    ) clear what the goal is: The goal is to write a wrapper around fgets()
    ) which strips the trailing newline. For historical reasons, "gets()"
    ) trims the terminating newline, fgets() doesn't.
    )
    ) Here's a first pass:
    )
    ) char *safegets(char *buf, int size) {
    ) int c;
    ) int count = 1;
    ) char *s = buf;
    ) if (s && size > 0) {
    ) while ((c = getchar()) != EOF &&
    ) c != '\n' &&
    ) count++ < size) {
    ) *s++ = c;
    ) }
    ) *s++ = '\0';
    ) return buf;
    ) } else {
    ) /* invalid arguments */
    ) return NULL;
    ) }
    ) }

    Now why in blazes did you suddenly decide to drop the fgets() ?

    With a little massaging, the fgets() version could be something like:

    char *safegets(char *buf, int size);
    {
    char *nl;
    if (buf == NULL || size < 1) {
    return NULL;
    }
    if (!fgets(buf, size, stdin)) {
    /* fgets failed */
    return NULL;
    }
    nl = strchr(buf, '\n');
    if (nl) {
    /* found a newline, trim it */
    *nl = '\0';
    }
    return buf;
    }

    Which looks clean as a whistle, and nicely separates
    the three steps the function is supposed to take:
    - sanity check inputs
    - read line
    - trim newline
    - return result.
    Four. The four steps the function is supposed to take. Ahem.


    SaSW, Willem
    --
    Disclaimer: I am in no way responsible for any of the statements
    made in the above text. For all I know I might be
    drugged or something..
    No I'm not paranoid. You all think I'm paranoid, don't you !
    #EOT
    Willem, Aug 25, 2010
    #11
  12. parjit

    Seebs Guest

    On 2010-08-25, Willem <> wrote:
    > Now why in blazes did you suddenly decide to drop the fgets() ?


    Because, IMHO, it's the wrong tool for the job -- specifically, it
    gives you no way to trim the trailing null without iterating over
    the entire string an extra time. It seemed to me that avoiding
    that would be an improvement, although obviously it's a debatable
    point.

    > Which looks clean as a whistle, and nicely separates
    > the three steps the function is supposed to take:
    > - sanity check inputs
    > - read line
    > - trim newline
    > - return result.
    > Four. The four steps the function is supposed to take. Ahem.


    AMONG the steps this function is supposed to take...

    -s
    --
    Copyright 2010, all wrongs reversed. Peter Seebach /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
    Seebs, Aug 25, 2010
    #12
  13. On 24 Aug, 22:35, Paul N <> wrote:
    > On 24 Aug, 21:51, parjit <> wrote:
    >
    >
    >
    >
    >
    > > Hi

    >
    > > I'm working on a small c project. I need to read lines from stdin, but
    > > because gets is known to be an undefined function I've been given the
    > > code below to use instead. It works fine but I'd like to understand what
    > > its doing and I just find it really confusing.

    >
    > > Can anyone explain to me what's going on here. Thanks.

    >
    > > #include <string.h>
    > > char *safegets(char *buf, int sz)
    > > {
    > >    char *r,*p;
    > >    return (buf==NULL || sz<1) ? NULL :
    > >      sz==1 ? *buf='\0', buf :
    > >      !(r=fgets(buf,sz,stdin)) ? r :
    > >      (p = strchr(buf, '\n')) ? *p=0, r : r;
    > > }

    >
    > I think whoever wrote that code may be messing you about.
    >
    > If you say something like
    >
    > return a ? b : c;
    >
    > then this is the same as:
    >
    > if (a) return b; else return c;
    >
    > and because "return" causes the following statements to be missed out,
    > this is the same as:
    >
    > if (a) return b;
    > return c;
    >
    > So applying that a few times and making a few other small changes, we
    > get:
    >
    > char *safegets(char *buf, int sz)
    > {
    > char *r,*p;
    > if (buf==NULL || sz<1) return NULL;
    > if (sz==1)  { buf[0]='\0'; return buf; }
    > r=fgets(buf,sz,stdin);
    > if (!r) return r;
    > p = strchr(buf, '\n');
    > if (p) { p[0]=0; return r; }
    > return r;
    >
    > }
    >
    > which in turn means:
    > bail out if buf is NULL or sz (size) is too small
    > return empty string if there's no space for anything longer
    > read in one line, or sz-1 characters, whichever is shorter
    > return NULL if that failed
    > look for a newline
    > replace it if you find one
    > return the string read (same as buf)
    >
    > Hope that helps.


    is you space bar broken?
    Nick Keighley, Aug 25, 2010
    #13
  14. parjit

    Jeff Clough Guest

    Seebs <> writes:

    > On Tue, 24 Aug 2010 18:57:33 -0600, Uno <> wrote:
    >>And, here, he's answering somebody's homework. This is the first
    >>worksheet the prof hands out.

    >
    > Is it? What university, or course? We could, of course verify this. I
    > didn't see anything obviously marking it as homework, and I have a really
    > hard time imagining that being the first of ANYTHING in a C course, because
    > you generally get through a lot of other stuff before you start working
    > with nested ?:.


    For what it's worth, I've seen exercises along these lines where the
    intro is basically "You've been assigned to fix a bug in some maniac's
    code." But something like that in the first week? Ehhhh...

    Jeff

    --
    web: http://www.chaosphere.com
    Author of Genesys, a Free Universal Paper and Pencil RPG.
    http://www.chaosphere.com/genesys/
    Jeff Clough, Aug 25, 2010
    #14
  15. Ian Collins <> writes:

    > On 08/25/10 08:51 AM, parjit wrote:

    <snip>
    >> Can anyone explain to me what's going on here. Thanks.
    >>
    >> #include<string.h>
    >> char *safegets(char *buf, int sz)
    >> {
    >> char *r,*p;
    >> return (buf==NULL || sz<1) ? NULL :
    >> sz==1 ? *buf='\0', buf :
    >> !(r=fgets(buf,sz,stdin)) ? r :
    >> (p = strchr(buf, '\n')) ? *p=0, r : r;
    >> }

    >
    > If the project contains code like this, run away!


    Actually I don't find the idea (a big nested conditional) as awful as so
    many other people seem to do. I hate the layout (indentation and lack of
    spaces) and it has some unnecessary complications (r, for example, and a
    pointless special case when sz == 1) but a nested conditional is not,
    itself, a monstrosity to me. Maybe too many years of Lisp perhaps...

    It's a shame that history has left us with strchr that returns NULL on
    failure. If it returned a pointer to the terminating null, then one
    could write *strchr(buf, '\n') = 0 (and that's not the only
    simplification it would yield). Still, there is always strcspn!.

    In the end, only one level of conditional is required:

    char *safegets(char *buf, int sz)
    {
    return !buf || sz < 1 || !fgets(buf, sz, stdin)
    ? NULL
    : (buf[strcspn(buf, "\n")] = 0, buf);
    }

    I think the result is reasonably clear. There are three simple reasons
    to return NULL. In all other cases the buffer pointer is returned after
    possibly replacing a newline with null.

    Of course, the curiosity is that I'd never write this. I can't imagine
    any project where the coding standard would consider this to be OK. The
    fact that I do is just a quirk.

    --
    Ben.
    Ben Bacarisse, Aug 25, 2010
    #15
  16. parjit

    Seebs Guest

    On 2010-08-25, Ben Bacarisse <> wrote:
    > Actually I don't find the idea (a big nested conditional) as awful as so
    > many other people seem to do.


    I don't find a big nested conditional utterly horrible.

    I do find the (*foo = bar, foo) type sub-expressions unacceptable.

    Too much conceptual nesting. I do not think conditional expressions should
    have conditional side-effects, that's just too error-prone.

    -s
    --
    Copyright 2010, all wrongs reversed. Peter Seebach /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
    Seebs, Aug 25, 2010
    #16
  17. parjit

    parjit Guest

    Seebs wrote:
    > On Tue, 24 Aug 2010 18:57:33 -0600, Uno <> wrote:
    >>And, here, he's answering somebody's homework. This is the first
    >>worksheet the prof hands out.

    >
    > Is it? What university, or course? We could, of course verify this. I
    > didn't see anything obviously marking it as homework, and I have a
    > really hard time imagining that being the first of ANYTHING in a C
    > course, because you generally get through a lot of other stuff before
    > you start working with nested ?:.


    It is not a homework, it is code the teacher gave us to use as a
    component in our homeworks to take care of error checking and unsafeness
    in standard functions. We do not have to explain this code but I want to
    understand it for my own interest. It is very complicated code, I don't
    think I will ever be able to write code this clever :(

    Here are the safe versions of malloc and free. Also confusing to me!

    void *safemalloc(int sz)
    {
    void *r;
    return sz<=0 ? 0 :
    (r=malloc(sz)) ? r :
    ((!!printf("fatal error cannot alocate %d bytes dumping core",sz)
    && (exit(1),0)),NULL);
    }

    void safefree(void *p)
    {
    free(p?p:
    (!!printf("fatal error cannot free null pointer dumping core")
    && (exit(1),0),NULL));
    }
    parjit, Aug 25, 2010
    #17
  18. parjit

    Paul N Guest

    On 25 Aug, 09:03, Nick Keighley <>
    wrote:
    > On 24 Aug, 22:35, Paul N <> wrote:
    >
    >
    >
    >
    >
    > > On 24 Aug, 21:51, parjit <> wrote:

    >
    > > > Hi

    >
    > > > I'm working on a small c project. I need to read lines from stdin, but
    > > > because gets is known to be an undefined function I've been given the
    > > > code below to use instead. It works fine but I'd like to understand what
    > > > its doing and I just find it really confusing.

    >
    > > > Can anyone explain to me what's going on here. Thanks.

    >
    > > > #include <string.h>
    > > > char *safegets(char *buf, int sz)
    > > > {
    > > >    char *r,*p;
    > > >    return (buf==NULL || sz<1) ? NULL :
    > > >      sz==1 ? *buf='\0', buf :
    > > >      !(r=fgets(buf,sz,stdin)) ? r :
    > > >      (p = strchr(buf, '\n')) ? *p=0, r : r;
    > > > }

    >
    > > I think whoever wrote that code may be messing you about.

    >
    > > If you say something like

    >
    > > return a ? b : c;

    >
    > > then this is the same as:

    >
    > > if (a) return b; else return c;

    >
    > > and because "return" causes the following statements to be missed out,
    > > this is the same as:

    >
    > > if (a) return b;
    > > return c;

    >
    > > So applying that a few times and making a few other small changes, we
    > > get:

    >
    > > char *safegets(char *buf, int sz)
    > > {
    > > char *r,*p;
    > > if (buf==NULL || sz<1) return NULL;
    > > if (sz==1)  { buf[0]='\0'; return buf; }
    > > r=fgets(buf,sz,stdin);
    > > if (!r) return r;
    > > p = strchr(buf, '\n');
    > > if (p) { p[0]=0; return r; }
    > > return r;

    >
    > > }

    >
    > > which in turn means:
    > > bail out if buf is NULL or sz (size) is too small
    > > return empty string if there's no space for anything longer
    > > read in one line, or sz-1 characters, whichever is shorter
    > > return NULL if that failed
    > > look for a newline
    > > replace it if you find one
    > > return the string read (same as buf)

    >
    > > Hope that helps.

    >
    > is you space bar broken?


    Er, no. I was trying to re-organise the original code to make it
    easier to understand. It didn't occur to me to go through it putting
    spaces in, to be honest, though code I had written myself would have
    more spaces. Even looking back, I think the lack of spaces is not
    really the code's biggest problem.

    AFAIK, the only problem with my system is the way blank lines appear
    before the last } of all my functions. See above, for instance. I
    think this is due to a problem with Google's quoting mechanism.

    Paul.
    Paul N, Aug 25, 2010
    #18
  19. parjit

    Jorgen Grahn Guest

    On Wed, 2010-08-25, parjit wrote:
    > Seebs wrote:
    >> On Tue, 24 Aug 2010 18:57:33 -0600, Uno <> wrote:
    >>>And, here, he's answering somebody's homework. This is the first
    >>>worksheet the prof hands out.

    >>
    >> Is it? What university, or course? We could, of course verify this. I
    >> didn't see anything obviously marking it as homework, and I have a
    >> really hard time imagining that being the first of ANYTHING in a C
    >> course, because you generally get through a lot of other stuff before
    >> you start working with nested ?:.

    >
    > It is not a homework, it is code the teacher gave us to use as a
    > component in our homeworks to take care of error checking and unsafeness
    > in standard functions. We do not have to explain this code but I want to
    > understand it for my own interest. It is very complicated code, I don't
    > think I will ever be able to write code this clever :(


    The code may have been clever, but it was *not* clever to give it to
    people without documentation. It's easy to explain, in a few
    sentences, what a gets() replacement does, and if the author has done
    that well I don't feel any need to review the code, even if it looks
    too clever. If he hasn't, I do not trust his judgement, and not his
    code either.

    > Here are the safe versions of malloc and free. Also confusing to me!
    >
    > void *safemalloc(int sz)
    > {
    > void *r;
    > return sz<=0 ? 0 :
    > (r=malloc(sz)) ? r :
    > ((!!printf("fatal error cannot alocate %d bytes dumping core",sz)
    > && (exit(1),0)),NULL);
    > }
    >
    > void safefree(void *p)
    > {
    > free(p?p:
    > (!!printf("fatal error cannot free null pointer dumping core")
    > && (exit(1),0),NULL));
    > }


    There are so many problems with that, I don't know where to start.
    I half suspect you are joking ... but anyway, to name just a few that
    I see:

    - no documentation (unless you omitted it)
    - he says "safe" but means "the program will exit", potentially
    losing data, killing people, or whatever. It's generally
    *less* safe than to allow the caller to handle the error.
    - ... and here the caller will have to handle some errors anyway
    (why exit for out-of-memory but not for safemalloc(-42)?)
    - exit(1) will never cause a core dump, not even on systems which
    support core dumps
    - int is the wrong argument to a malloc(); it should be size_t
    - free(0) is safe (and useful!), but safefree(0) is not.
    - lots of the weird subexpressions (e.g. !!x) seem useless to me,
    but I haven't bothered to decode them.

    I think you should continue the course, but not trust what the teacher
    says. You have internet access, so you can study good C code, do
    exercises and read this group between classes.

    /Jorgen

    --
    // Jorgen Grahn <grahn@ Oo o. . .
    \X/ snipabacken.se> O o .
    Jorgen Grahn, Aug 25, 2010
    #19
  20. parjit

    Seebs Guest

    On 2010-08-25, parjit <> wrote:
    > It is not a homework, it is code the teacher gave us to use as a
    > component in our homeworks to take care of error checking and unsafeness
    > in standard functions. We do not have to explain this code but I want to
    > understand it for my own interest. It is very complicated code, I don't
    > think I will ever be able to write code this clever :(


    Ugh.

    No offense, but your teacher sucks at teaching C.

    > Here are the safe versions of malloc and free. Also confusing to me!


    These are the same kind of trickery.

    > void *safemalloc(int sz)
    > {
    > void *r;
    > return sz<=0 ? 0 :
    > (r=malloc(sz)) ? r :
    > ((!!printf("fatal error cannot alocate %d bytes dumping core",sz)
    > && (exit(1),0)),NULL);
    > }


    I would refuse to pay money to learn C from someone who wrote this. I count
    three obvious errors plus a bewildering variety of crappy design choices and
    badly-written code.

    > void safefree(void *p)
    > {
    > free(p?p:
    > (!!printf("fatal error cannot free null pointer dumping core")
    > && (exit(1),0),NULL));
    > }


    Same here, plus it's actually less safe than the standard C one, because
    free(NULL) has been well-defined for something over twenty years.

    I wish you the best of luck in your course. Know that you will be learning
    C despite this idiot, not because of him.

    -s
    --
    Copyright 2010, all wrongs reversed. Peter Seebach /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
    Seebs, Aug 25, 2010
    #20
    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. Tony Johansson
    Replies:
    1
    Views:
    296
    Peter Julian
    May 21, 2005
  2. Slain
    Replies:
    5
    Views:
    362
    James Kanze
    Jul 18, 2007
  3. jake

    Help understanding some C code

    jake, Oct 30, 2009, in forum: C Programming
    Replies:
    12
    Views:
    820
    Chris M. Thomasson
    Nov 1, 2009
  4. N. Demos

    Need help understanding some JS code.

    N. Demos, Nov 20, 2005, in forum: Javascript
    Replies:
    3
    Views:
    145
  5. Thomas Foster

    I need help understanding some code..

    Thomas Foster, Mar 5, 2014, in forum: Ruby
    Replies:
    3
    Views:
    152
    Robert Klemme
    Mar 7, 2014
Loading...

Share This Page