this calls and checks strtol() correctly?

[lovecreatesbea...]

Does this part of C code call and check strtol() correctly?

port = strtol(argv[2], &endptr, 10);
if (argv[2] == endptr){
fprintf(stderr, "%s\n", "Invalid port number form");
return 1;
}
if (port == 0 && errno == EINVAL){
perror("strtol()");
return 1;
}
if ((port == LONG_MAX || port == LONG_MIN) && errno == ERANGE){
perror("strtol()");
return 1;
}

Thank you for your time

[badc0de4]

[e-mail] wrote:

Does this part of C code call and check strtol() correctly?

Reset errno before calling strtol().

errno = 0;

port = strtol(argv[2], &endptr, 10);
if (argv[2] == endptr){
fprintf(stderr, "%s\n", "Invalid port number form");
return 1;
}
if (port == 0 && errno == EINVAL){
perror("strtol()");
return 1;
}
if ((port == LONG_MAX || port == LONG_MIN) && errno == ERANGE){
perror("strtol()");
return 1;
}

Thank you for your time

You might want to add a test for *endptr != '\0'
If argv[2] contains "443foo", *endptr will be 'f' after the strtol()
call (and port will be 433)

[Harald van =?UTF-8?b?RMSzaw==?=]

On Sun, 29 Jun 2008 01:34:53 -0700, [e-mail] wrote:

Does this part of C code call and check strtol() correctly?

Probably not.

port = strtol(argv[2], &endptr, 10);

strtol won't set errno to 0 if there's no error. You need to do that
yourself before calling it. I'll pretend you did set it to 0 when for the
rest.

if (argv[2] == endptr){

endptr == argv[2] only argv[2] doesn't start with a number. If it does,
for example if argv[2] is "10xab", then endptr will point to the 'x'. You
probably don't want to consider that a valid number.

fprintf(stderr, "%s\n", "Invalid port number form");
return 1;
}
if (port == 0 && errno == EINVAL){

The check may fail to compile on systems that don't define EINVAL, and on
those that do, the block will never be entered.

perror("strtol()");
return 1;
}
if ((port == LONG_MAX || port == LONG_MIN) && errno == ERANGE){

This is not invalid but you can simplify it to checking if
errno == ERANGE. If it's set to that, you already know that
(port == LONG_MAX || port == LONG_MIN).

perror("strtol()");
return 1;
}

Thank you for your time

[Tor Rustad]

[e-mail] wrote:

Does this part of C code call and check strtol() correctly?

port = strtol(argv[2], &endptr, 10);
if (argv[2] == endptr){
fprintf(stderr, "%s\n", "Invalid port number form");
return 1;
}
if (port == 0 && errno == EINVAL){
perror("strtol()");
return 1;
}
if ((port == LONG_MAX || port == LONG_MIN) && errno == ERANGE){
perror("strtol()");
return 1;
}

Thank you for your time

Not tested yet, but here is a function I wrote a couple of days ago, to
do the same:

/**
\fn int xatoi(const char *str)
safe atoi() replacement

\remark exit on failure
*/
extern int xatoi(const char *str)
{
long l;
char *endptr;

assert(str != NULL);

errno = 0;
l = strtol(str, &endptr, 10);

/* handle EINVAL and ERANGE */
if (errno) {
fprintf(stderr,
"Error: conversion of '%s' to type int failed,
%s\n",
str,
strerror(errno));
exit(EXIT_FAILURE);
}
/* Not a number? */
if (endptr == str) {
fprintf(stderr,
"Error: conversion of '%s' failed, not a
number\n", str);
exit(EXIT_FAILURE);
}
/* require next char after number to be in [ \0\n\t\r] */
assert(*endptr == '\0' || *endptr == ' ' || *endptr == '\n' ||
*endptr == '\r' || *endptr == '\t' );

/* check for over- and underflow */
assert(l <= INT_MAX);
assert(l >= INT_MIN);

return (int) l;
}

--
Tor <bw****@wvtqvm.vw | tr i-za-h a-z>

[Roland Pibinger]

On Sun, 29 Jun 2008 01:51:34 -0700 (PDT), ba******@gmail.com wrote:

>You might want to add a test for *endptr != '\0'
If argv[2] contains "443foo", *endptr will be 'f' after the strtol()
call (and port will be 433)

In general (not for argv[2]) you may want to accept trailing
whitespace, e.g. because strtol accepts leading whitespace.
--
Roland Pibinger
"The best software is simple, elegant, and full of drama" - Grady Booch

[lovecreatesbea...]

On Jun 29, 5:01 pm, Harald van D?k <[e-mail]:

On Sun, 29 Jun 2008 01:34:53 -0700, [e-mail] wrote:

Does this part of C code call and check strtol() correctly?

Probably not.

port = strtol(argv[2], &endptr, 10);

strtol won't set errno to 0 if there's no error. You need to do that
yourself before calling it. I'll pretend you did set it to 0 when for the
rest.

if (argv[2] == endptr){

endptr == argv[2] only argv[2] doesn't start with a number. If it does,
for example if argv[2] is "10xab", then endptr will point to the 'x'. You
probably don't want to consider that a valid number.

fprintf(stderr, "%s\n", "Invalid port number form");
return 1;
}
if (port == 0 && errno == EINVAL){

The check may fail to compile on systems that don't define EINVAL, and on
those that do, the block will never be entered.

perror("strtol()");
return 1;
}
if ((port == LONG_MAX || port == LONG_MIN) && errno == ERANGE){

This is not invalid but you can simplify it to checking if
errno == ERANGE. If it's set to that, you already know that
(port == LONG_MAX || port == LONG_MIN).

perror("strtol()");
return 1;
}

Thank you. I come up with this new one.

errno = 0;
port = strtol(argv[2], &endptr, 10);
if (errno == ERANGE){
perror("strtol() bbb");
return 1;
}
if (endptr == argv[2]){
fprintf(stderr, "%s\n", "Invalid form");
return 1;
}
if (*endptr != '\0'){
while (endptr != argv[2] + strlen(argv[2])){
if (!isspace(*endptr++)){
fprintf(stderr, "%s\n", "Invalid
form");
return 1;
}
}
}

[lovecreatesbea...]

On Jun 29, 7:39*pm, [e-mail] (Roland Pibinger) wrote:

On Sun, 29 Jun 2008 01:51:34 -0700 (PDT), [e-mail] wrote:

You might want to add a test for *endptr != '\0'
If argv[2] contains "443foo", *endptr will be 'f' after the strtol()
call (and port will be 433)

In general (not for argv[2]) you may want to accept trailing
whitespace, e.g. because strtol accepts leading whitespace.

Only " 443 " is considered valid in the code in my last post, while
both "443foo" and "443 foo" are invalid.

[Barry Schwarz]

On Sun, 29 Jun 2008 07:50:17 -0700 (PDT),
"[e-mail]" <lo***************@gmail.comwrote:

snip

>Thank you. I come up with this new one.

errno = 0;
port = strtol(argv[2], &endptr, 10);

port must be at least as large as a long. If it is an int or shorter,
you could still have undefined behavior depending on the value of
argv[2].

if (errno == ERANGE){
perror("strtol() bbb");
return 1;
}
if (endptr == argv[2]){
fprintf(stderr, "%s\n", "Invalid form");
return 1;
}
if (*endptr != '\0'){

You don't really need this if statement. Should *endptr equal
'\0', then endptr must equal argv[2]+strlen(argv[2]). In this case,
the following while statement will evaluate to false and the loop will
not be entered.

while (endptr != argv[2] + strlen(argv[2])){
if (!isspace(*endptr++)){
fprintf(stderr, "%s\n", "Invalid
form");
return 1;
}
}
}

Remove del for email

[lovecreatesbea...]

On Jun 30, 6:01 am, Barry Schwarz <[e-mail]:

On Sun, 29 Jun 2008 07:50:17 -0700 (PDT),

"[e-mail]" <[e-mail]:

errno = 0;
port = strtol(argv[2], &endptr, 10);

port must be at least as large as a long. If it is an int or shorter,
you could still have undefined behavior depending on the value of
argv[2].

Yes, it's defined as the type of the return type of strtol().

if (errno == ERANGE){
perror("strtol() bbb");
return 1;
}
if (endptr == argv[2]){
fprintf(stderr, "%s\n", "Invalid form");
return 1;
}
if (*endptr != '\0'){

You don't really need this if statement. Should *endptr equal
'\0', then endptr must equal argv[2]+strlen(argv[2]). In this case,
the following while statement will evaluate to false and the loop will
not be entered.

while (endptr != argv[2] + strlen(argv[2])){
if (!isspace(*endptr++)){
fprintf(stderr, "%s\n", "Invalid
form");
return 1;
}
}
}

Thank you.

I made it have more indent with that outer if statement. Removing it
seems better.