crepeatdefensive-programming

Replacing `goto` with a different programming construct


I m trying to do this little programm with defensive programming but its more than difficult for me to handle this avoiding the Loop-Goto as i know that as BAD programming. I had try with while and do...while loop but in one case i dont have problem. Problem begins when i m going to make another do...while for the second case ("Not insert space or click enter button"). I tried and nested do...while but here the results was more complicated.

#include <ctype.h>
#include <stdlib.h>
#include <string.h>

int main()
{
    int i;
    int length;
    char giventext [25];        
    Loop:

    printf("String must have 25 chars lenght:\n");
    gets(giventext);

    length = strlen(giventext);

    if (length > 25) {
        printf("\nString has over %d chars.\nMust give a shorter string\n", length);
        goto Loop;
    }
    /* Here i trying to not give space or nothing*/
    if (length < 1) {
        printf("You dont give anything as a string.\n");
        goto Loop;
    } else {
        printf("Your string has %d\n",length);
        printf("Letter in lower case are: \n");

        for (i = 0; i < length; i++) {
            if (islower(giventext[i])) {                            
                printf("%c",giventext[i]);
            }
        }
    }
    return 0;
}

Solution

  • Note that your code is not defensive at all. You have no way to avoid a buffer overflow because,

    1. you check for the length of the string after it has been input to your program so after the buffer overflow has already occurred and
    2. you used gets() which doesn't check input length and thus is very prone to buffer overflow.

    Use fgets() instead and just discard extra characters.

    I think you need to understand that strlen() doesn't count the number of characters of input but instead the number of characters in a string.

    If you want to ensure that there are less than N characters inserted then

    int
    readinput(char *const buffer, int maxlen)
    {
        int count;
        int next;
    
        fputc('>', stdout);
        fputc(' ', stdout);
    
        count = 0;
        while ((next = fgetc(stdin)) && (next != EOF) && (next != '\n')) {
            // We need space for the terminating '\0';
            if (count == maxlen - 1) {
                // Discard extra characters before returning
                // read until EOF or '\n' is found
                while ((next = fgetc(stdin)) && (next != EOF) && (next != '\n'))
                    ;
                return -1;
            }
            buffer[count++] = next;
        }
        buffer[count] = '\0';
        return count;
    }
    
    int
    main(void)
    {
        char string[8];
        int result;
    
        while ((result = readinput(string, (int) sizeof(string))) == -1) {
            fprintf(stderr, "you cannot input more than `%d' characters\n", 
                                (int) sizeof(string) - 1);
        }
        fprintf(stdout, "accepted `%s' (%d)\n", string, result);
    }
    

    Note that by using a function, the flow control of this program is clear and simple. That's precisely why goto is discouraged, not because it's an evil thing but instead because it can be misused like you did.