getting a warning about gets

Discussion in 'C Programming' started by pereges, May 19, 2008.

  1. pereges

    pereges Guest

    I'm getting a warning on my linux gcc compiler:

    /tmp/ccXgHa9s.o(.text+0x48): In function `main':
    : warning: the `gets' function is dangerous and should not be used.

    And here's where I used gets:

    #include "common.h" //common.h includes string.h
    ....
    ....

    int main(void)
    {
    char zf_name[50];

    ....
    printf("Enter the name of the file\n");
    gets(zf_name);
    ...
    return 0;
    }
     
    pereges, May 19, 2008
    #1
    1. Advertising

  2. pereges

    Guest

    On May 19, 10:38 am, pereges <> wrote:
    > I'm getting a warning on my linux gcc compiler:
    >
    > /tmp/ccXgHa9s.o(.text+0x48): In function `main':
    > : warning: the `gets' function is dangerous and should not be used.
    >
    > And here's where I used gets:
    >
    > #include "common.h"  //common.h includes string.h
    > ...
    > ...
    >
    > int main(void)
    > {
    >    char zf_name[50];
    >
    >    ....
    >    printf("Enter the name of the file\n");
    >    gets(zf_name);
    >    ...
    >    return 0;
    >
    >
    >
    > }- Hide quoted text -
    >
    > - Show quoted text -


    I think the message is rather explicit; gets() is dangerous, and
    shouldn't be used.

    Alternatively, use fgets() or perhaps, scanf() to get input

    --
    include
     
    , May 19, 2008
    #2
    1. Advertising

  3. pereges

    viza Guest

    Hi

    On May 19, 11:38 am, pereges <> wrote:

    > : warning: the `gets' function is dangerous and should not be used.
    > char zf_name[50];
    > gets(zf_name);



    How do you know the file name is less than 50 characters?

    Use:
    char zf_name[ SOME_CONSTANT ];
    fgets( zf_name, SOME_CONSTANT, stdin );

    This will at least not overflow your buffer.

    You should also do:

    if( ferror( stdin )) /* there was a read error */

    if( zf_name[ strlen(zf_name) - 1 ] == '\n' )
    zf_name[ strlen(zf_name) - 1 ] = 0; /* take the newline character
    off the filename */

    else
    /* line was longer than SOME_CONSTANT, and part of it is missing */


    and maybe also:

    if( feof( stdin )) /* end of file was reached */


    HTH
    viza
     
    viza, May 19, 2008
    #3
  4. pereges

    Guest

    On May 19, 1:54 pm, viza <> wrote:
    > Hi
    >
    > On May 19, 11:38 am, pereges <> wrote:
    >
    > > : warning: the `gets' function is dangerous and should not be used.
    > > char zf_name[50];
    > > gets(zf_name);

    >
    > How do you know the file name is less than 50 characters?
    >
    > Use:
    > char zf_name[ SOME_CONSTANT ];
    > fgets( zf_name, SOME_CONSTANT, stdin );
    >
    > This will at least not overflow your buffer.
    >
    > You should also do:
    >
    > if( ferror( stdin )) /* there was a read error */

    Does not necessarily mean there was a read error. It could have been
    that EOF was reached.
    if(!feof(stdin) && ferror(stdin)) means an error happened.
    > if( zf_name[ strlen(zf_name) - 1 ] == '\n' )

    First check that strlen(zf_name) > 0
    > zf_name[ strlen(zf_name) - 1 ] = 0; /* take the newline character
    > off the filename */
    >
    > else
    > /* line was longer than SOME_CONSTANT, and part of it is missing */

    Not necessarily again. Most of the times that's what it means. If EOF
    wasn't reached, you can try to read a byte from stdin with getc. If
    getc returns EOF, check the stream if feof(). If feof() returns a
    positive value, then the line wasn't longer than SOME_CONSTANT (unless
    ferror() returns positive). If getc() does not return EOF, ungetc the
    return value, and in that case yes; the line was longer than
    SOME_CONSTANT.
     
    , May 19, 2008
    #4
  5. pereges

    Flash Gordon Guest

    viza wrote:
    > Hi
    >
    > On May 19, 11:38 am, pereges <> wrote:
    >
    >> : warning: the `gets' function is dangerous and should not be used.
    >> char zf_name[50];
    >> gets(zf_name);

    >
    >
    > How do you know the file name is less than 50 characters?
    >
    > Use:
    > char zf_name[ SOME_CONSTANT ];
    > fgets( zf_name, SOME_CONSTANT, stdin );
    >
    > This will at least not overflow your buffer.
    >
    > You should also do:
    >
    > if( ferror( stdin )) /* there was a read error */


    Why not check the value returned by fgets which will catch both an input
    error and an end-of-file? Then, if you need to distinguish between them
    can either ferror or feof.

    If there was either an error or an end-of-file you want to avoid
    executing the code below.

    > if( zf_name[ strlen(zf_name) - 1 ] == '\n' )
    > zf_name[ strlen(zf_name) - 1 ] = 0; /* take the newline character
    > off the filename */
    >
    > else
    > /* line was longer than SOME_CONSTANT, and part of it is missing */
    >
    >
    > and maybe also:
    >
    > if( feof( stdin )) /* end of file was reached */


    Well, that would need to be done up above when checking ferror rather
    than down here (which is probably what you intended but might not be
    obviouse to the OP).
    --
    Flash gordon
     
    Flash Gordon, May 19, 2008
    #5
  6. pereges

    pereges Guest

    Will this suffice:

    if(scanf("%s", zf_name) != 1)
    {
    perror("Erroneous file name!");
    exit(EXIT_FAILURE);
    }


    Or should I also check if the array limit is not exceeded ?
     
    pereges, May 19, 2008
    #6
  7. On May 19, 5:03 am, pereges <> wrote:
    > Will this suffice:
    >
    > if(scanf("%s", zf_name) != 1)


    As coded, this is no better than fgets. There are modifiers you can
    place between the % and the s which will prevent buffer overflow but
    it becomes even harder to determine if you received the entire file
    name. fgets is still the easiest to use.

    > {
    >     perror("Erroneous file name!");
    >     exit(EXIT_FAILURE);
    >
    > }
    >
    > Or should I also check if the array limit is not exceeded ?


    You cannot check this after the fact. Once the limit is exceeded you
    are in the realm of undefined behavior.
     
    Barry Schwarz, May 19, 2008
    #7
  8. pereges

    pereges Guest

    On May 19, 5:23 pm, Barry Schwarz <> wrote:
    > On May 19, 5:03 am, pereges <> wrote:
    >
    > > Will this suffice:

    >
    > > if(scanf("%s", zf_name) != 1)

    >
    > As coded, this is no better than fgets. There are modifiers you can
    > place between the % and the s which will prevent buffer overflow but
    > it becomes even harder to determine if you received the entire file
    > name. fgets is still the easiest to use.
    >
    > > {
    > > perror("Erroneous file name!");
    > > exit(EXIT_FAILURE);

    >
    > > }

    >
    > > Or should I also check if the array limit is not exceeded ?

    >
    > You cannot check this after the fact. Once the limit is exceeded you
    > are in the realm of undefined behavior.


    I agree. What about the following code ?

    if(fgets(zf_name, MAX_SIZE, stdin) == NULL)
    {
    if(ferror(stdin))
    {
    perror("Error while reading file name!");
    exit(EXIT_FAILURE);
    }
    if(feof(stdin))
    {
    perror("End of file reached!");
    exit(EXIT_FAILURE);
    }
    }

    Also I'm wondering about the new line character which makes fgets
    stops reading but is included in the string copied to zf_name.
    Shouldn't we deal with it too ?
     
    pereges, May 19, 2008
    #8
  9. pereges

    Guest

    On May 19, 4:33 pm, pereges <> wrote:
    > On May 19, 5:23 pm, Barry Schwarz <> wrote:
    >
    >
    >
    > > On May 19, 5:03 am, pereges <> wrote:

    >
    > > > Will this suffice:

    >
    > > > if(scanf("%s", zf_name) != 1)

    >
    > > As coded, this is no better than fgets. There are modifiers you can
    > > place between the % and the s which will prevent buffer overflow but
    > > it becomes even harder to determine if you received the entire file
    > > name. fgets is still the easiest to use.

    >
    > > > {
    > > > perror("Erroneous file name!");
    > > > exit(EXIT_FAILURE);

    >
    > > > }

    >
    > > > Or should I also check if the array limit is not exceeded ?

    >
    > > You cannot check this after the fact. Once the limit is exceeded you
    > > are in the realm of undefined behavior.

    >
    > I agree. What about the following code ?
    >
    > if(fgets(zf_name, MAX_SIZE, stdin) == NULL)
    > {
    > if(ferror(stdin))
    > {
    > perror("Error while reading file name!");
    > exit(EXIT_FAILURE);
    > }
    > if(feof(stdin))
    > {
    > perror("End of file reached!");
    > exit(EXIT_FAILURE);
    > }
    >
    > }
    >
    > Also I'm wondering about the new line character which makes fgets
    > stops reading but is included in the string copied to zf_name.
    > Shouldn't we deal with it too ?

    All your questions are answered in my previous post.
     
    , May 19, 2008
    #9
  10. writes:
    > On May 19, 1:54 pm, viza <> wrote:

    [...]
    >> if( ferror( stdin )) /* there was a read error */

    > Does not necessarily mean there was a read error. It could have been
    > that EOF was reached.


    Um, yes, ferror(stdin) does mean that there was a read error.

    > if(!feof(stdin) && ferror(stdin)) means an error happened.


    That means that either an error or an end-of-file condition happened.

    [snip]

    --
    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, May 19, 2008
    #10
  11. Richard Heathfield <> writes:
    > Keith Thompson said:
    >> writes:
    >>> On May 19, 1:54 pm, viza <> wrote:

    >> [...]
    >>>> if( ferror( stdin )) /* there was a read error */
    >>> Does not necessarily mean there was a read error. It could have been
    >>> that EOF was reached.

    >>
    >> Um, yes, ferror(stdin) does mean that there was a read error.
    >>
    >>> if(!feof(stdin) && ferror(stdin)) means an error happened.

    >>
    >> That means that either an error or an end-of-file condition happened.

    >
    > It does? I'd have thought that it meant that both an error happened and an
    > end-of-file condition /didn't/ happen. What am I missing?


    Nothing. I just wasn't paying attention. Whoops.

    --
    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, May 19, 2008
    #11
  12. pereges

    Guest

    On May 19, 7:20 pm, pete <> wrote:
    > wrote:
    > > On May 19, 1:54 pm, viza <> wrote:

    >
    > >> if( ferror( stdin )) /* there was a read error */

    > > Does not necessarily mean there was a read error.

    >
    > That's wrong.
    >
    > > It could have been that EOF was reached.
    > > if(!feof(stdin) && ferror(stdin)) means an error happened.

    >
    > I'm guessing that you don't know
    > what the return value of feof means either.
    >
    > N869
    > 7.19.10.2 The feof function
    > Returns
    > [#3] The feof function returns nonzero if and only if the
    > end-of-file indicator is set for stream.
    >
    > 7.19.10.3 The ferror function
    > Returns
    > [#3] The ferror function returns nonzero if and only if the
    > error indicator is set for stream.

    Oh! I did not know about that. Thanks a lot. I wonder just how much of
    my C knowledge is wrong.
    Regardless, the OP should still use both feof and ferror to find out
    which happened when getc returned EOF.
     
    , May 19, 2008
    #12
  13. pereges

    pereges Guest

    On May 19, 10:02 pm, wrote:

    > Oh! I did not know about that. Thanks a lot. I wonder just how much of
    > my C knowledge is wrong.
    > Regardless, the OP should still use both feof and ferror to find out
    > which happened when getc returned EOF.


    where did getc come into picture ? i guess its a part of fgets
    implementation ?
     
    pereges, May 19, 2008
    #13
  14. Barry Schwarz wrote:
    > pereges <> wrote:
    > > Will this suffice:
    > >
    > > if(scanf("%s", zf_name) != 1)

    >
    > As coded, this is no better than fgets. ...


    YM gets().

    --
    Peter
     
    Peter Nilsson, May 19, 2008
    #14
    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. Stefan Mueller
    Replies:
    5
    Views:
    541
    Steven Saunderson
    Jul 10, 2006
  2. Replies:
    10
    Views:
    505
  3. John Joyce

    gets gets

    John Joyce, Mar 26, 2007, in forum: Ruby
    Replies:
    2
    Views:
    374
    John Joyce
    Mar 26, 2007
  4. John Joyce

    Return of gets gets

    John Joyce, Apr 23, 2007, in forum: Ruby
    Replies:
    0
    Views:
    210
    John Joyce
    Apr 23, 2007
  5. libsfan01
    Replies:
    5
    Views:
    265
    Jeff North
    Dec 20, 2006
Loading...

Share This Page