Problem reading two strings with getchar() and then printing those strings in C

Question

This is my code for two functions in C:

// Begin

void readTrain(Train_t *train){


    printf("Name des Zugs:");
    char name[STR];
    getlinee(name, STR);
    strcpy(train->name, name);

    printf("Name des Drivers:");
    char namedriver[STR];
    getlinee(namedriver, STR);
    strcpy(train->driver, namedriver);

}

void getlinee(char *str, long num){

    char c;
    int i = 0;

    while(((c=getchar())!='\n') && (i<num)){
        *str = c;
        str++;
        i++;
    }

    printf("i is %d\n", i);

    *str = '\0';

    fflush(stdin);
}

// End

So, with void getlinee(char *str, long num) function I want to get user input to first string char name[STR] and to second char namedriver[STR]. Maximal string size is STR (30 charachters) and if I have at the input more than 30 characters for first string ("Name des Zuges"), which will be stored in name[STR], after that I input second string, which will be stored in namedriver, and then printing FIRST string, I do not get the string from the user input (first 30 characters from input), but also the second string "attached" to this, I simply do not know why...otherwise it works good, if the limit of 30 characters is respected for the first string. Here my output, when the input is larger than 30 characters for first string, problem is in the row 5 "Zugname", why I also have second string when I m printing just first one...:

Name des Zugs:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

i is 30

Name des Drivers:xxxxxxxx

i is 8

Zugname: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaxxxxxxxx

Drivername: xxxxxxxx

Answer 1

I think your issue is that your train->name is not properly terminated with '\0', as a consequence when you call printf("%s", train->name) the function keeps reading memory until it finds '\0'. In your case I guess your structure looks like:

struct Train_t {
    //...
    char name[STR];
    char driver[STR];
    //...
};

In getlinee() function, you write '\0' after the last character. In particular, if the input is more than 30 characters long, you copy the first 30 characters, then add '\0' at the 31-th character (name[30]). This is a first buffer overflow.

So where is this '\0' actually written? well, at name[30], even though your not supposed to write there. Then, if you have the structure above when you do strcpy(train->name, name); you will actually copy a 31-bytes long string: 30 chars into train->name, and the '\0' will overflow into train->driver[0]. This is the second buffer overflow.

After this, you override the train->driver buffer so the '\0' disappears and your data in memory basically looks like:

train->name = "aaa...aaa" // no '\0' at the end so printf won't stop reading here
train->driver = "xxx\0"   // but there

Answer 2

I believe you have a struct similar to this:

typedef struct train_s
{
    //...
    char        name[STR];
    char        driver[STR];
   //...
}               Train_t;

When you attempt to write a '\0' to a string that is longer than STR (30 in this case), you actually write a '\0' to name[STR], which you don't have, since the last element of name with length STR has an index of STR-1 (29 in this case), so you are trying to write a '\0' outside your array.

And, since two strings in this struct are stored one after another, you are writing a '\0' to driver[0], which you immediately overwrite, hence when printing out name, printf doesn't find a '\0' until it reaches the end of driver, so it prints both.

Fixing this should be easy.

Just change:

 while(((c=getchar())!='\n') && (i<num))

to:

 while(((c=getchar())!='\n') && (i<num - 1))

Or, as I would do it, add 1 to array size:

char name[STR + 1];
char driver[STR + 1];

Answer 3

You have an off-by-one error on your array sizes -- you have arrays of STR chars, and you read up to STR characters into them, but then you store a NUL terminator, requiring (up to) STR + 1 bytes total. So whenever you have a max size input, you run off the end of your array(s) and get undefined behavior.

Pass STR - 1 as the second argument to getlinee for the easiest fix.

Answer 4

Key issues

Size test in wrong order and off-by-one. ((c=getchar())!='\n') && (i<num) --> (i+1<num) && ((c=getchar())!='\n'). Else no room for the null character. Bad form to consume an excess character here.

getlinee() should be declared before first use. Tip: Enable all compiler warnings to save time.

---

Other

Use int c; not char c; to well distinguish the typical 257 different possible results from getchar().

fflush(stdin); is undefined behavior. Better code would consume excess characters in a line with other code.

void getlinee(char *str, long num) better with size_t num. size_t is the right size type for array sizing and indexing.

int i should be the same type as num.

Better code would also test for EOF.

while((i<num) && ((c=getchar())!='\n') && (c != EOF)){

A better design would return something from getlinee() to indicate success and identify troubles like end-of-file with nothing read, input error, too long a line and parameter trouble like str == NULL, num <= 0.