Depending on which platform you are using, the line
students[x].firstName=(char*)malloc(sizeof(char*));
is most likely equivalent to either
students[x].firstName = malloc( 4 );
or
students[x].firstName = malloc( 8 );
because that is the size of a char *
(pointer to char
) on most platforms.
This means that if the user input is more than 3 or 7 characters (4 or 8 including the terminating null character), then the lines
printf("Enter first name :");
scanf("%s",students[x].firstName);
will cause a buffer overflow, which will invoke undefined behavior. This means that your program could crash or misbehave in some other way.
Therefore, your concerns are valid. The line
students[x].firstName=(char*)malloc(sizeof(char*));
should instead specify a size that is sufficient to store the maximum possible length of the user input, instead of only the size of a pointer (which is 4 or 8 bytes).
However, even if you specify a much higher value instead, such as
students[x].firstName = malloc( 200 );
then your program still is not safe, because the user could still cause a buffer overflow by entering a word with a length of at least 200 characters. Therefore, it would be safer to write
scanf( "%199s", students[x].firstName );
instead of:
scanf("%s",students[x].firstName);
This will limit the number of characters written to the memory buffer by scanf
to 200 (199 matched characters plus the terminating null character).
However, even if this solution is better, it still is not ideal, because scanf
will silently truncate the input, if it is too long, and will leave the rest of the line on the input stream. This means that this left-over input will likely cause trouble the next time you call scanf
on the input stream, because that is what will be read first.
For this reason, it would be better to always read an entire line of input, and if it is too long to be stored in the memory buffer, to reject the input with an error message and to prompt the user for new input.
In the code below is an example that uses fgets
instead of scanf
, because fgets
is better suited for reading whole lines of input. However, since using fgets
is also not that easy, I am using fgets
in two helper functions that I created myself. I call these functions get_line_from_user
and get_int_from_user
. Those are the two functions that I call from main
. I don't call fgets
directly from main
.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <limits.h>
#include <errno.h>
//forward declarations
void get_line_from_user( const char prompt[], char buffer[], int buffer_size );
int get_int_from_user( const char *prompt );
#define NUM_STUDENTS 2
#define MAX_STRING_SIZE 200
int main( void )
{
typedef struct
{
char* firstName;
char* lastName;
int rollNumber;
} STUDENT;
STUDENT* students = malloc( NUM_STUDENTS * sizeof *students );
//fill student data from user
for ( int i = 0; i < NUM_STUDENTS; i++)
{
students[i].firstName = malloc( MAX_STRING_SIZE );
get_line_from_user(
"Enter first name: ",
students[i].firstName,
MAX_STRING_SIZE
);
//shrink the allocated memory buffers to the
//required size
students[i].firstName = realloc(
students[i].firstName,
strlen( students[i].firstName ) + 1
);
students[i].lastName = malloc( MAX_STRING_SIZE );
get_line_from_user(
"Enter last name: ",
students[i].lastName,
MAX_STRING_SIZE
);
students[i].rollNumber =
get_int_from_user( "Enter roll number: " );
//shrink the allocated memory buffers to the
//required size
students[i].lastName = realloc(
students[i].lastName,
strlen( students[i].lastName ) + 1
);
}
//print back the stored input
for ( int i = 0; i < NUM_STUDENTS; i++)
printf(
"First Name: %s, Last Name: %s, Roll number: %d\n",
students[i].firstName, students[i].lastName,
students[i].rollNumber
);
return EXIT_SUCCESS;
}
//This function will read exactly one line of input from the
//user. It will remove the newline character, if it exists. If
//the line is too long to fit in the buffer, then the function
//will automatically reprompt the user for input. On failure,
//the function will never return, but will print an error
//message and call "exit" instead.
void get_line_from_user( const char prompt[], char buffer[], int buffer_size )
{
for (;;) //infinite loop, equivalent to while(1)
{
char *p;
//prompt user for input
fputs( prompt, stdout );
//attempt to read one line of input
if ( fgets( buffer, buffer_size, stdin ) == NULL )
{
printf( "Error reading from input!\n" );
exit( EXIT_FAILURE );
}
//attempt to find newline character
p = strchr( buffer, '\n' );
//make sure that entire line was read in (i.e. that
//the buffer was not too small to store the entire line)
if ( p == NULL )
{
int c;
//a missing newline character is ok if the next
//character is a newline character or if we have
//reached end-of-file (for example if the input is
//being piped from a file or if the user enters
//end-of-file in the terminal itself)
if ( (c=getchar()) != '\n' && !feof(stdin) )
{
if ( ferror(stdin) )
{
printf( "Error reading from input!\n" );
exit( EXIT_FAILURE );
}
printf( "Input was too long to fit in buffer!\n" );
//discard remainder of line
do
{
c = getchar();
if ( ferror(stdin) )
{
printf( "Error reading from input!\n" );
exit( EXIT_FAILURE );
}
} while ( c != '\n' && c != EOF );
//reprompt user for input by restarting loop
continue;
}
}
else
{
//remove newline character by overwriting it with
//null character
*p = '\0';
}
//input was ok, so break out of loop
break;
}
}
//This function will attempt to read one integer from the user. If
//the input is invalid, it will automatically reprompt the user,
//until the input is valid.
int get_int_from_user( const char *prompt )
{
//loop forever until user enters a valid number
for (;;)
{
char buffer[1024], *p;
long l;
//prompt user for input
fputs( prompt, stdout );
//get one line of input from input stream
if ( fgets( buffer, sizeof buffer, stdin ) == NULL )
{
fprintf( stderr, "Unrecoverable input error!\n" );
exit( EXIT_FAILURE );
}
//make sure that entire line was read in (i.e. that
//the buffer was not too small)
if ( strchr( buffer, '\n' ) == NULL && !feof( stdin ) )
{
int c;
printf( "Line input was too long!\n" );
//discard remainder of line
do
{
c = getchar();
if ( c == EOF )
{
fprintf( stderr, "Unrecoverable error reading from input!\n" );
exit( EXIT_FAILURE );
}
} while ( c != '\n' );
continue;
}
//attempt to convert string to number
errno = 0;
l = strtol( buffer, &p, 10 );
if ( p == buffer )
{
printf( "Error converting string to number!\n" );
continue;
}
//make sure that number is representable as an "int"
if ( errno == ERANGE || l < INT_MIN || l > INT_MAX )
{
printf( "Number out of range error!\n" );
continue;
}
//make sure that remainder of line contains only whitespace,
//so that input such as "6abc" gets rejected
for ( ; *p != '\0'; p++ )
{
if ( !isspace( (unsigned char)*p ) )
{
printf( "Unexpected input encountered!\n" );
//cannot use `continue` here, because that would go to
//the next iteration of the innermost loop, but we
//want to go to the next iteration of the outer loop
goto continue_outer_loop;
}
}
return l;
continue_outer_loop:
continue;
}
}
This program has the following behavior:
Enter first name: This line is longer than 200 characters. This line is longer than 200 characters. This line is longer than 200 characters. This line is longer than 200 characters. This line is longer than 200 characters.
Input was too long to fit in buffer!
Enter first name: John
Enter last name: Doe
Enter roll number: 8
Enter first name: Jane
Enter last name: Doe
Enter roll number: 9
First Name: John, Last Name: Doe, Roll number: 8
First Name: Jane, Last Name: Doe, Roll number: 9
Note that it is generally recommended to always check the return value of malloc
, because otherwise, if malloc
fails for some reason, then the program will misbehave. I did not add these checks to my code, because I did not want to distract from the other changes I made.