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]

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 Dijk]

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).

 

[Roland Pibinger]

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.

 

[lovecreatesbea...]

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...]

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),

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...]

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.