Thread: Question about reading in strings and integers

I have some problems with your readint() function. For starters, atoi() is generally considered obsolete and it is recommended you use strtol() instead, which checks for an int overflow:

If the string has valid syntax for an integer but the value is not representable because of overflow, strtol returns either LONG_MAX or LONG_MIN (see Range of Type), as appropriate for the sign of the value. It also sets errno to ERANGE to indicate there was overflow.

Also, your own method for doing this is not good. Converting back into a string and comparing the lengths will not work if, for example, the value "wraps around"* creating a negative number, because including the minus sign the string could be the same length. For example, 4147483648 will become -147483648 if converted into a (32 bit) signed int. As strings, both those have 10 characters. You could, however, reliably use strcmp() here, in which case you would not have to check anything else (if you insist on using atoi).

Furthermore, testing an int to see if it exceeds the minimum or maximum value an int can hold is oxymoronic. It is impossible for an int to return a value outside this range -- that is what the range indicates. Get it? Because of this and the way you have arranged your conditions here,

Code:

if(tmpbuflen != buflen || INT_MIN < i && i > INT_MAX)

This function will NEVER return an error, because the final condition says "AND i is greater than INT_MAX". That is impossible, so the if will always fail. I think perhaps you meant && to be ||, but in any case as demonstrated this is an irrelevant test.

*which is what an int overflow does (altho in the case of a string conversion it may not literally be a wrap around the effect is similar). An int overflow is not like a buffer overflow, it does not overwrite the bounds of the int, it just results in an arithmetically incorrect result (or in this case an incorrect string converstion).

I didn't even even notice the fflush(stdin)! That is undefined by the C standard and should never be used.

You may want to read this too:
STDIN pitfalls

@MK27
Thanks for your response. After researching strtol I see the benefit of using it. Also I get to accomplish what I set out to do, actually verify what is being read in is in an int range by reading it to a long and then use INT_MIN and INT_MAX to verify it's a proper integer. It also gave me an opportunity to learn about errno.h (which I had no clue it existed). Also thanks for the link about fflush, I had no idea it was for flushing output only. I have a bunch of tutorials that use it rather haphazardly. Guess it's back to K&R for me.

@seaking1
Thanks for pointing out my lack of optimization. I was being a little lazy, I was doing this when I was supposed to be working on a JavaScript GUI library. I love C! It's so much fun, well so far anyways. Also thanks for the great link.

One other question, if a name or something should be a particular size, is my way of handling this consistent with what you would see in other C programs or should I flag it bad if it exceeds expectations?

Here is the code update, any more suggestions?

Code:

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

void read(char *out, const int size);
int readint(int *out);

int main(void)
{
	char buffer[17] = {0};
	int i = 0;
	//double d = 0;
	printf("read string[16]: ");
	read(buffer, 17);
	
	printf("read integer: ");
	while(!readint(&i)) {
		printf("the number you entered is beyond the range allowable, please try again.");
	}

	//printf("read double: ");
	//readdouble(&d);
	
	printf("read: %s\t\t%d\n\n", buffer, strlen(buffer));
	printf("readint: %d\n\n", i);
	//printf("readint: %5.2f\n\n", d);
	
	system("PAUSE");
	return 0;
}

/**
	function:	read
	purpose:	reads in a string and assigns it to out. Requires a length of 
				string parameter of n-1 (-1 allows for null terminator).
	args:		char *out, const int strlen
	returns:	void
*/
void read(char *out, const int strlen)
{
	char ch;
	int char_count = 0;

	// Read in the keyboard input until enter is pressed or
	// we exceed the buffer. We will not allow the buffer
	// to overflow by restricting what actually gets written
	// to the pointer.
	ch = getchar();
	*(out)++ = ch;

	while((ch = getchar()) != '\n' && ch != EOF)
		if(++char_count < strlen-1) // If we are outside of the range of chars to read, read the rest of characters in stdin
			*(out)++ = ch;

	// Null terminate the pointer so it is a proper string
	*out = '\0';
}

/**
	function:	readint
	purpose:	reads an integer and assigns it to the out pointer parameter.
	defines:	INT_LEN
	args:		int *out
	returns:	int (1 success, 0 error)
*/
#define	INT_LEN	100
int readint(int *out)
{
	long int val;
	char str[INT_LEN];		/* buffer for retriving an int */
	char **endptr = {'\0'};	/* end ptr for strtol */

	read(str, INT_LEN);		/* read input from the user as a string */

	/* convert the string to a long and ensure it didn't
	   wrap/exceed the bounds of an integer or the conversion 
	   could not be performed (such as reading a string) */
	val = strtol(str, endptr, 0);

	if(errno == ERANGE || val == 0 && INT_MIN < val > INT_MAX) {
		errno = 0;	/* reset the error code */
		return 0;
	}

	*out = (int)val;
	return 1;
}

C books recommendations