By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
458,122 Members | 1,598 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 458,122 IT Pros & Developers. It's quick & easy.

Sufficient error checking of strtol()?

P: n/a
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
Nov 14 '05 #1
Share this Question
Share on Google+
5 Replies


P: n/a
"William Payne" <mi******************@student.liu.se> 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.
Nov 14 '05 #2

P: n/a
nrk
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
Nov 14 '05 #3

P: n/a

"Thomas Pfaff" <th**********@tiscali.no> wrote in message
news:87************@tiscali.no...
"William Payne" <mi******************@student.liu.se> 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
Nov 14 '05 #4

P: n/a
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.

--
Er*********@sun.com
Nov 14 '05 #5

P: n/a
On Mon, 02 Feb 2004 23:13:45 GMT, nrk <ra*********@devnull.verizon.net>
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
Nov 14 '05 #6

This discussion thread is closed

Replies have been disabled for this discussion.