Reputation: 133
#include <stdio.h>
int main(void)
{
char str[10];int i,length=0;
line:
{
printf("Enter string of length 10 \n");
scanf("%[A-Za-z]s", str);
}
for (i=0; str[i] != '\0'; i++)
{
length++;
}
if (length != 10)
{
printf("Length of string is : %d \n Give a string of length 10 please \n", length);
goto line;
}
else
{
printf("length = %d \n", length);
printf("Your String is: %s", str);
}
return 0;
}
If the user inputs a string of length not equal to 10, then my expected output should be : printing out "Enter string o length 10" and waiting for the user to input another string and then check it's length.
I have removed Goto statement and checked for string of length not equal to 10, it does compute the length and outputs it, something like this :
Enter string of length 10
dciciibiyciyigiy
Length of string is : 16
Give a string of length 10 please.
*** stack smashing detected ***: ./palindrome.out terminated
Aborted (core dumped)
I have done similar Goto command execution in some other code elsewhere, but for this one : if I try to input a string of length not equal to 10, it shows the length and then goes into an infinite loop o printing something like this :
...
Length of string is : 428115
Give a string of length 10 please.
Enter string of length 10
Length of string is : 428130
Give a string of length 10 please.
Enter string of length 10
Length of string is : 428145
Give a string of length 10 please.
...
and so on. I do not understand what am I doing wrong?
Upvotes: 1
Views: 1190
Reputation: 123596
To hold a 10-character string, str
needs to be at least 11 elements wide:
char str[11]; // holds up to 10 characters plus 0 terminator
Unfortunately, the %s
specifier doesn't know how big the target buffer is - if you enter more characters than the target buffer is sized to hold, those extra characters will be written past the end of the buffer, potentially clobbering important data. If you want to limit the number of characters read from the input stream into str
, then you can add a field width to the format:
scanf( "%10s", str );
Unfortunately, unlike printf
, you can't specify the field with as an argument - it must be hardcoded into the format string. There is a way around it using macros:
#define MAX_INPUT_LENGTH 10 // or whatever value
#define STR(x) #x // "stringify" the argument
#define EXP(x) STR(x) // expand and then stringify the argument
#define FMT STR(%) EXP(MAX_INPUT_LENGTH) STR(s) // will expand to "%" "10" "s"
So you can write
char str[MAX_INPUT_LENGTH + 1]; // +1 for terminator
scanf( FMT, str );
and not worry about writing past end of str
.
Now, about that goto
...
There is a much better way to do this that doesn't use goto
at all, and is a little easier to follow:
#include <string.h> // for strlen
...
char str[MAX_INPUT_LENGTH + 1];
size_t len = 0;
do
{
printf( "Enter a string that's 10 characters long: " );
scanf( FMT, str );
} while ( strlen( str ) != 10 );
Now, as written, this has a few problems. First, if you enter more than MAX_INPUT_LENGTH
characters, the unread characters are left in the input stream, ready to foul up the next read. You would probably want to clear out any unread characters before asking for more:
do
{
printf( "Enter a string that's 10 characters long: " );
scanf( FMT, str );
while ( getchar() != '\n' ) // consume unread characters up to the newline
; // empty loop
} while ( strlen( str ) != 10 );
After scanf
reads MAX_INPUT_LENGTH
characters into str
, it then scrubs any extra characters up to the newline character.
But there's one more problem - you won't get a friendly error message if the string length isn't right - you'll just get the original prompt again. If you want to write the error message, you'll need to do another check within the body of the do
loop:
do
{
printf( "Enter a string that's 10 characters long: " );
scanf( FMT, str );
while ( getchar() != '\n' )
; // empty loop
if ( strlen( str ) != 10 )
printf( "Input is not the right length, try again.\n" );
} while ( strlen( str ) != 10 );
Calling strlen
twice is a bit ugly, so we can save the result of the first strlen
call to use again. To use it as part of the loop control expression, it must be declared outside of the loop body:
size_t len = 0;
do
{
printf( "Enter a string that's 10 characters long: " );
scanf( FMT, str );
while ( getchar() != '\n' )
; // empty loop
if ( (len = strlen( str )) != 10 ) // assign as part of the check
printf( "Input is not the right length, try again.\n" );
} while ( len != 10 );
Edit
Should have mentioned, you can avoid the buffer overflow concerns and macro trickery by using fgets
instead of scanf
:
size_t len = 0;
do
{
printf( "Enter a string that's 10 characters long: " );
if ( fgets( str, sizeof str, stdin ) )
{
while ( getchar() != '\n' )
; // empty loop
if ( (len = strlen( str )) != 10 ) // assign as part of the check
printf( "Input is not the right length, try again.\n" );
}
else
{
// EOF or error on input, handle as appropriate
}
} while ( len != 10 );
To check str
for non-alphabetic input, use the strpbrk
function:
#define NON_ALPHA "0123456789!@#$%^&*()-_=+[]{};:,.<>/?'\\|\"";
...
size_t len = 0;
char *non_alpha_ptr = NULL;
do
{
printf( "Enter a string that's 10 characters long: " );
if ( fgets( str, sizeof str, stdin ) )
{
while ( getchar() != '\n' )
; // empty loop
if ( (len = strlen( str )) != 10 ) // assign as part of the check
printf( "Input is not the right length, try again.\n" );
else if ( (non_alpha_ptr = strpbrk( str, NON_ALPHA ) ) )
printf( "Input is not strictly alphabetic, try again.\n" );
}
else
{
// EOF or error on input, handle as appropriate
}
} while ( len != 10 || non_alpha_ptr != NULL );
Upvotes: 2
Reputation: 437
Most important problem with your program is that you are allocating memory for
10 char
's (10 bytes) in str
, but scanf
is storing 11 char
's (11 bytes) (10 char
's which you enter + 1 sentinel/delimiter char '\0'
, that it inserts).
Hence the result of following line --
if (length != 10)
can be true the first time even if you enter only 10 char
's, and once it gets true for the first time, there is no looking back and length
is only going to increase as there is nothing that decreases length
in the goto
loop. Hence the goto
loop will loop forever.
Upvotes: 0
Reputation: 225827
You are not properly restricting how many characters the user can enter.
The array str
can hold 10 bytes, but the call to scanf
doesn't restrict the user from entering more characters. As a result, it is possible to write past the end of the array if too many characters are entered. This invokes undefined behavior, which in this case causes the code to crash.
You need to change the format specifier to scanf
to allow a maximum of 9 characters (because you need to save 1 byte for the null terminating character):
scanf("%9[A-Za-z]", str);
You can then get rid of the length check (and the associated goto
) since a string of 10 or longer can no longer be entered.
If you want the user to be able to enter up to 10 characters, you need increase the size of str
to 11.
Upvotes: 5