Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Sufficient error checking of strtol()?

Reply
Thread Tools

Sufficient error checking of strtol()?

 
 
William Payne
Guest
Posts: n/a
 
      02-02-2004
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


 
Reply With Quote
 
 
 
 
Thomas Pfaff
Guest
Posts: n/a
 
      02-02-2004
"William Payne" <(E-Mail Removed)> 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.
 
Reply With Quote
 
 
 
 
nrk
Guest
Posts: n/a
 
      02-02-2004
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
 
Reply With Quote
 
Eric Sosman
Guest
Posts: n/a
 
      02-02-2004
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.

--
http://www.velocityreviews.com/forums/(E-Mail Removed)
 
Reply With Quote
 
William Payne
Guest
Posts: n/a
 
      02-02-2004

"Thomas Pfaff" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> "William Payne" <(E-Mail Removed)> 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


 
Reply With Quote
 
AJ Mohan Rao
Guest
Posts: n/a
 
      02-03-2004
On Mon, 02 Feb 2004 23:13:45 GMT, nrk <(E-Mail Removed)>
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
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Is a 1/1.8" (7.18 x 5.32 mm) sensor sufficient for 10mp and 12mp? Paul D. Sullivan Digital Photography 67 02-17-2007 04:48 PM
Is a static method sufficient to call it factory? f.vivoli@gmail.com Java 11 07-29-2005 09:02 AM
Is AVG Anti-Virus sufficient? Sseaott Computer Support 20 07-01-2004 12:08 AM
newbie - PIX 501 sufficient Kevin Laro Cisco 6 05-25-2004 05:47 PM
system lacked sufficient buffer space ali Java 0 08-18-2003 07:58 AM



Advertisments