User Input

[Tarique]

Hello All.
Is the following good enough to be a safe user input routine?
What else should i do to improve it ?

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#define ARRAYSIZE 100

/*Clear the input stream before any input */
int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc(f))) && (EOF != ch))
continue;

return ch;
}

char* get_data( void )
{
static char buffer[ARRAYSIZE];
printf("Enter Data : ");

/*flushln(stdin);*/
while( fgets( buffer , sizeof buffer , stdin ) == NULL )
{
puts("Enter a data");
flushln(stdin);
fgets( buffer , sizeof buffer , stdin );
}
buffer[ strlen(buffer)-1 ] = '\0';

return buffer;
}


int main(void)
{
int iValue;
float fValue;
double dValue;
char *cValue;

iValue = atoi( get_data() );
fValue = (float)atof( get_data() );
dValue = atof( get_data() );
cValue = get_data();

printf( "Integer :%d \n" , iValue );
printf( "Float :%f \n" , fValue );
printf( "Double :%f \n" , dValue );
printf( "String :%s \n" , cValue );

return EXIT_SUCCESS;
}

 

[hltbra]

It's so safe and not so correct.

I don't think it's good to use a static buffer, you left memory
locked.
It's not so good. I guess it's better you use pointer and malloc.

Now, pay attention on this line:
buffer[ strlen(buffer)-1 ] = '\0';

And if I don't enter a newline as last character? You delete my last
character; fgets() agrees a EOF as end condition.
So it would be better to write:
char *p;
if ((p= strchr(buffer, '\n') != NULL)
*p = '\0';

Take a look at the atoi()'s man page, it doesn't care about errors.
See the strtol()'s man page too.
The same is valid for atof().

Beside these things, I think it's all right.

Happy hacking!

 

[hltbra]

Sorry, I made a mistake.
It's not so safe and not so correct.

 

[Keith Thompson]

Is the following good enough to be a safe user input routine?
What else should i do to improve it ?

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#define ARRAYSIZE 100

Style point: add a space between "include" and "<".

/*Clear the input stream before any input */
int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc(f))) && (EOF != ch))
continue;

return ch;
}

char* get_data( void )
{
static char buffer[ARRAYSIZE];
printf("Enter Data : ");

Call fflush(stdout) to ensure that the prompt appears.

/*flushln(stdin);*/
while( fgets( buffer , sizeof buffer , stdin ) == NULL )
{
puts("Enter a data");
flushln(stdin);
fgets( buffer , sizeof buffer , stdin );
}
buffer[ strlen(buffer)-1 ] = '\0';

return buffer;
}

You have some duplicated code here. The two prompts are different; is
that deliberate or accidental?

fgets() returns NULL on failure, which can occur due to reaching
end-of-file or an I/O error. In either case, trying again is unlikely
to work.

You don't allow for fgets() returning because the buffer is full
rather than because it read an entire line. Instead, you assume that
the last character of the string is '\n', and overwrite it with '\0'.

int main(void)
{
int iValue;
float fValue;
double dValue;
char *cValue;

iValue = atoi( get_data() );

atoi() doesn't detect errors; if the argument is "FOOBAR", it quietly
returns 0. Consider using strtol() and handling any errors.

fValue = (float)atof( get_data() );

atof() has the same error-checking problems as atoi(). Also, the cast
is unnecessary; atof() returns a double, but it will be implicitly
converted to float by a simple assignment.

dValue = atof( get_data() );
cValue = get_data();

printf( "Integer :%d \n" , iValue );
printf( "Float :%f \n" , fValue );
printf( "Double :%f \n" , dValue );
printf( "String :%s \n" , cValue );

return EXIT_SUCCESS;
}

How is the user supposed to know that he needs to enter an integer
value, two-floating-point values, and a string in response to the four
identical prompts?

 

[Eric Sosman]

Hello All.
Is the following good enough to be a safe user input routine?

No. Well, it depends on what you think is "good enough,"
but under any definition of "good enough" that's good enough
to be called good, I'd say the answer is No.

What else should i do to improve it ?

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#define ARRAYSIZE 100

/*Clear the input stream before any input */
int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc(f))) && (EOF != ch))
continue;

return ch;
}

char* get_data( void )
{
static char buffer[ARRAYSIZE];
printf("Enter Data : ");

See Question 12.4 in the comp.lang.c Frequently Asked

/*flushln(stdin);*/
while( fgets( buffer , sizeof buffer , stdin ) == NULL )
{

It's not at all obvious that "Just keep trying" is a valid
strategy here. fgets() has already informed you that it was
unable to read; why do you think a second attempt will do any
better? Quite plausibly, the program will just spin in this
loop, prompting endlessly and getting nowhere until somebody
loses patience and pulls the plug.

puts("Enter a data");
flushln(stdin);
fgets( buffer , sizeof buffer , stdin );

And what happens to the two lines that are consumed here
(one by flushln and one by fgets)? Suppose the user does in
fact "Enter a data" when asked; what happens to that data?

}
buffer[ strlen(buffer)-1 ] = '\0';

What happens if the line had more than ARRAYSIZE-1 characters
before the newline? Answer: You (1) don't notice that the whole
line isn't there, and (2) throw away the last of the characters
that you actually managed to read. Problem (2) may also occur on
the last line of a file, if it doesn't end with a newline.

return buffer;
}

General note: The error-handling is not well thought out.
For example, flushln is careful to check for EOF and to return
EOF if it's detected, but what does get_data do with the alert
that flushln returns to it? Nothing at all! It just ignores
the problem and plows blindly ahead. That's like installing
smoke alarms and taking the batteries out to reduce the noise.

There are many ways to skin the read-a-line cat; some are
found at <http://www.cpax.org.uk/prg/writings/fgetdata.php>.
You needn't necessarily use any of the approaches described
there, but studying them should at least give you ideas about
how you might improve your own.