Correct way of taking in two integers

[Steve Carter]

The following code fragment prompts the user to enter two integers
separated by a space. It works the way I want it to, but a friend
said it's not correct and said it can cause a buffer overflow. I
don't really see anything wrong with it. Can someone enlighten me?
int main(void)
{
int i,j;
char buff[50];

printf("Enter two integers: ");
fflush(stdout);
fgets(buff,50,stdin);

if (sscanf(buff,"%d%d",&i,&j) != 2) {
printf("\nYou must enter 2 integers!\n");
return -1;
}

printf("\n%d + %d = %d\n",i,j,i+j);

return 0;
}

[Alf P. Steinbach]

* Steve Carter:

The following code fragment prompts the user to enter two integers
separated by a space. It works the way I want it to, but a friend
said it's not correct and said it can cause a buffer overflow. I
don't really see anything wrong with it. Can someone enlighten me?

int main(void)
{
int i,j;
char buff[50];

printf("Enter two integers: ");
fflush(stdout);
fgets(buff,50,stdin);

if (sscanf(buff,"%d%d",&i,&j) != 2) {
printf("\nYou must enter 2 integers!\n");
return -1;
}

printf("\n%d + %d = %d\n",i,j,i+j);

return 0;
}

It's not that it can cause a buffer overflow in the sense of storing
data beyond the end of the buffer, but that the buffer isn't large
enough, and can never be large enough, to tackle all input that is
otherwise valid but contains too many spaces.

Why don't you use fscanf, and dispense with the buffer?

Additional advice: in this interactive program it doesn't matter, but
it's a good habit to always present error messages on the standard error
stream, instead of the standard output stream; also, although -1 is an
old convention, EXIT_FAILURE is slightly more portable and clear.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[attn.steven.kuo]

Steve Carter wrote:

The following code fragment prompts the user to enter two integers
separated by a space. It works the way I want it to, but a friend
said it's not correct and said it can cause a buffer overflow. I
don't really see anything wrong with it. Can someone enlighten me?
int main(void)
{
int i,j;
char buff[50];

printf("Enter two integers: ");
fflush(stdout);
fgets(buff,50,stdin);

if (sscanf(buff,"%d%d",&i,&j) != 2) {
printf("\nYou must enter 2 integers!\n");
return -1;
}

printf("\n%d + %d = %d\n",i,j,i+j);

return 0;
}

What if the value represented by the string of digits
exceeds the maximum limit of int?

You maywant to try strtol instead.

For example:

#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
#include <errno.h>

#define MYBUFSIZE 50

extern int snprintf(char *, size_t, const char *, ...);

int main (void)
{
signed int value;
long int try_strtol;
char buf[MYBUFSIZE];
if ( snprintf(buf, MYBUFSIZE, "%lu", ULONG_MAX) >= MYBUFSIZE )
{
fprintf(stderr, "snprintf: value truncated\n");
return EXIT_FAILURE;
}
if (sscanf(buf, "%d", &value) != 1)
return EXIT_FAILURE;

printf("Buffer contains %s\n", buf);
printf("Scanned value %d\n", value);
errno = 0;
try_strtol = strtol(buf, NULL, 0);
if (errno == ERANGE)
{
if (try_strtol == LONG_MAX )
fprintf(stderr, "Overflow while trying to convert %s to
long\n", buf);
return EXIT_FAILURE;
}

return EXIT_SUCCESS;
}
For example, on my system the
above program produces:

Buffer contains 4294967295
Scanned value 2147483647
Overflow while trying to convert 4294967295 to long

--
Hope this helps,
Steven

[Keith Thompson]

"Alf P. Steinbach" <al***@start.nowrites:

* Steve Carter:

>The following code fragment prompts the user to enter two integers
separated by a space. It works the way I want it to, but a friend
said it's not correct and said it can cause a buffer overflow. I
don't really see anything wrong with it. Can someone enlighten me?
int main(void)
{
int i,j;
char buff[50];
printf("Enter two integers: ");
fflush(stdout);
fgets(buff,50,stdin);
if (sscanf(buff,"%d%d",&i,&j) != 2) {
printf("\nYou must enter 2 integers!\n");
return -1;
}
printf("\n%d + %d = %d\n",i,j,i+j);
return 0;
}

It's not that it can cause a buffer overflow in the sense of storing
data beyond the end of the buffer, but that the buffer isn't large
enough, and can never be large enough, to tackle all input that is
otherwise valid but contains too many spaces.

So if the user enters

123 456

he might get an error message. That doesn't seem too bad, though I'd
probably use a buffer bigger than 50 characters.

Why don't you use fscanf, and dispense with the buffer?

For one thing, fscanf skips all whitespace -- including newlines.

The real problem is that if the input contains valid integers, but
their value exceeds INT_MAX, the behavior is undefined. (Most likely
you'll just get a garbabe value, but in principle anything can
happen.)

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Richard Bos]

Keith Thompson <ks***@mib.orgwrote:

"Alf P. Steinbach" <al***@start.nowrites:

Why don't you use fscanf, and dispense with the buffer?

For one thing, fscanf skips all whitespace -- including newlines.

The real problem is that if the input contains valid integers, but
their value exceeds INT_MAX, the behavior is undefined. (Most likely
you'll just get a garbabe value, but in principle anything can
happen.)

Garbage values are bad enough. They could easily look reasonable to your
error checking code, but be entirely wrong. Corrupt but innocent-looking
data is a bugger to detect, filter and correct.

Richard