Sufficient error checking of strtol()?

Discussion in 'C Programming' started by William Payne, Feb 2, 2004.

  1. Hello, I am in the process of converting a C++ program to a C program. The
    user of the program is supposed to supply an integer on the command line and
    in the C++ version of the program I was using something called stringstreams
    to do the conversion. Here's my C version, can I leave it as it is or does
    it need to be robustified or changed in any manner, regarding error
    checking?

    char* endptr;

    errno = 0;

    unsigned long port_number = strtol(argv[2], &endptr, 0);

    if(strcmp("", endptr) != 0)
    {
    fprintf(stderr, "Couldn't convert %s to a valid port number.\n",
    argv[2]);

    return EXIT_FAILURE;
    }

    if(errno == ERANGE)
    {
    fprintf(stderr,
    "Value %s is too big or small to fit in an unsigned long.\n",
    argv[2]);

    return EXIT_FAILURE;
    }

    Thanks for any replies!

    / William Payne
     
    William Payne, Feb 2, 2004
    #1
    1. Advertising

  2. William Payne

    Thomas Pfaff Guest

    "William Payne" <> writes:

    > char* endptr;
    > errno = 0;


    Make sure argv[2] is valid and non-NULL

    if (argc > 2)

    > unsigned long port_number = strtol(argv[2], &endptr, 0);


    strtol returns a long. Use strtoul for unsigned long.

    > if(strcmp("", endptr) != 0)


    if (*endptr != '\0')

    > {
    > fprintf(stderr, "Couldn't convert %s to a valid port number.\n",
    > argv[2]);

    [...]

    The rest looks fine to me.
     
    Thomas Pfaff, Feb 2, 2004
    #2
    1. Advertising

  3. William Payne

    nrk Guest

    William Payne wrote:

    > Hello, I am in the process of converting a C++ program to a C program. The
    > user of the program is supposed to supply an integer on the command line
    > and in the C++ version of the program I was using something called
    > stringstreams to do the conversion. Here's my C version, can I leave it as
    > it is or does it need to be robustified or changed in any manner,
    > regarding error checking?
    >
    > char* endptr;
    >
    > errno = 0;
    >
    > unsigned long port_number = strtol(argv[2], &endptr, 0);
    >
    > if(strcmp("", endptr) != 0)
    > {
    > fprintf(stderr, "Couldn't convert %s to a valid port number.\n",
    > argv[2]);
    >
    > return EXIT_FAILURE;
    > }
    >


    Using strcmp here is overkill. I would prefer:
    if ( endptr == argv[2] || *endptr )

    The first condition above checks for the case of the empty string (OT: I
    can't come up with a way of passing in empty strings from my shell (bash)
    [note that exec* calls are excluded from this discussion]).

    > if(errno == ERANGE)
    > {
    > fprintf(stderr,
    > "Value %s is too big or small to fit in an unsigned
    > long.\n", argv[2]);
    >


    Probably would use strerror to get a localized error message and then create
    my custom one from that.

    -nrk.

    > return EXIT_FAILURE;
    > }
    >
    > Thanks for any replies!
    >
    > / William Payne


    --
    Remove devnull for email
     
    nrk, Feb 2, 2004
    #3
  4. William Payne

    Eric Sosman Guest

    William Payne wrote:
    >
    > Hello, I am in the process of converting a C++ program to a C program. The
    > user of the program is supposed to supply an integer on the command line and
    > in the C++ version of the program I was using something called stringstreams
    > to do the conversion. Here's my C version, can I leave it as it is or does
    > it need to be robustified or changed in any manner, regarding error
    > checking?
    >
    > char* endptr;
    >
    > errno = 0;
    >
    > unsigned long port_number = strtol(argv[2], &endptr, 0);


    Since you want an `unsigned long' result, you should
    probably use strtoul() instead of strtol(). Otherwise, you'll
    get possibly unwelcome behavior for input like "-3".

    Also, give careful consideration to the third argument. As
    you've written it, an input like "010" will produce the value eight
    rather than ten. If that's what you want, fine -- but if you think
    your users may be more familiar with decimal numbers than with C's
    source conventions for octal, you might want to change it.

    > if(strcmp("", endptr) != 0)
    > {
    > fprintf(stderr, "Couldn't convert %s to a valid port number.\n",
    > argv[2]);
    >
    > return EXIT_FAILURE;
    > }


    You've got a design decision here: Should you allow trailing
    whitespace? Note that you've implicitly allowed leading whitespace
    and signs by using strto*().

    If you choose to treat all "leftovers" as errors, I'd suggest
    testing `if (*endptr != '\0')' instead of calling strcmp().

    You might also want to do some usage-specific range checking.
    For example, there might be reason to reject 1073741823 or zero
    as valid "port numbers."

    > if(errno == ERANGE)
    > {
    > fprintf(stderr,
    > "Value %s is too big or small to fit in an unsigned long.\n",
    > argv[2]);
    >
    > return EXIT_FAILURE;
    > }
    >
    > Thanks for any replies!


    It's usually recommended to clear zero `errno' before the
    call, just in case it happened to be holding the value `ERANGE'
    beforehand.

    Putting all this together, I'd suggest something like

    errno = 0;
    port_number = strtoul(argv[2], &endptr, 10);
    if (endptr == argv[2])
    ...; /* no conversion at all */
    else if (*endptr != '\0')
    ...; /* incomplete conversion */
    else if (errno == ERANGE)
    ...; /* out of `unsigned long' range */
    else if (port_number < lowest || port_number > highest)
    ...; /* valid `unsigned long' but invalid port */
    else
    ...; /* whoopee! */

    Of course, you could collapse some of these cases if you don't
    need to discriminate between them. Also, you could check
    `isdigit( (unsigned char) argv[2][0] )' if you want to reject
    leading whitespace and/or signs.

    --
     
    Eric Sosman, Feb 2, 2004
    #4
  5. "Thomas Pfaff" <> wrote in message
    news:...
    > "William Payne" <> writes:
    >
    > > char* endptr;
    > > errno = 0;

    >
    > Make sure argv[2] is valid and non-NULL
    >
    > if (argc > 2)
    >
    > > unsigned long port_number = strtol(argv[2], &endptr, 0);

    >
    > strtol returns a long. Use strtoul for unsigned long.
    >
    > > if(strcmp("", endptr) != 0)

    >
    > if (*endptr != '\0')
    >
    > > {
    > > fprintf(stderr, "Couldn't convert %s to a valid port number.\n",
    > > argv[2]);

    > [...]
    >
    > The rest looks fine to me.


    Thanks for your reply Thomas. I already had an argc check in my program, but
    I forgot to mention it in the post. The user has to supply two arguments in
    fact, a hostname and a port number so the first thing I do is check if argc
    < 3 and if it's less than three I exit with an error message. Do I have to
    check something else before passing argv[2] to strtoul()? The
    strtol()/strtoul() confusion arises from the fact that I realised when
    writing my original post that only non-negative port numbers are valid in my
    program so I changed from strtol() to strtoul(). I will get rid of the
    strcmp() and do what you suggested instead.

    // WP
     
    William Payne, Feb 2, 2004
    #5
  6. William Payne

    AJ Mohan Rao Guest

    On Mon, 02 Feb 2004 23:13:45 GMT, nrk <>
    wrote:

    > William Payne wrote:
    >
    >> Hello, I am in the process of converting a C++ program to a C program.
    >> The
    >> user of the program is supposed to supply an integer on the command line
    >> and in the C++ version of the program I was using something called
    >> stringstreams to do the conversion. Here's my C version, can I leave it
    >> as
    >> it is or does it need to be robustified or changed in any manner,
    >> regarding error checking?
    >>
    >> char* endptr;
    >>
    >> errno = 0;
    >>
    >> unsigned long port_number = strtol(argv[2], &endptr, 0);
    >>
    >> if(strcmp("", endptr) != 0)
    >> {
    >> fprintf(stderr, "Couldn't convert %s to a valid port number.\n",
    >> argv[2]);
    >>
    >> return EXIT_FAILURE;
    >> }
    >>

    >
    > Using strcmp here is overkill. I would prefer:
    > if ( endptr == argv[2] || *endptr )
    >
    > The first condition above checks for the case of the empty string (OT: I
    > can't come up with a way of passing in empty strings from my shell (bash)


    $ ./a.out ""

    (*argv[1] == 0) is true

    --
    Mohan
     
    AJ Mohan Rao, Feb 3, 2004
    #6
    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. ali
    Replies:
    0
    Views:
    4,474
  2. Replies:
    11
    Views:
    704
  3. Sai Kumar
    Replies:
    3
    Views:
    353
    Jack Klein
    Aug 7, 2004
  4. Muhammad Alkarouri

    Is __mul__ sufficient for operator '*'?

    Muhammad Alkarouri, Oct 20, 2009, in forum: Python
    Replies:
    7
    Views:
    450
    Mick Krippendorf
    Oct 26, 2009
  5. Parrot
    Replies:
    4
    Views:
    200
    Hal Rosser
    Dec 15, 2004
Loading...

Share This Page