Replacing gets() with fgets()

Question

I've been testing this struct out and I get warning about using gets. Somebody mentioned to use fgets instead and replace the end with '\0'. Any recommendation how I can change my code to do that?

void regCars(Car reg[], int *pNrOfCars) {
    char again[WORDLENGTH] = "yes", model[WORDLENGTH], tmp[WORDLENGTH];
    int year, milage;

    while (strcmp(again, "yes") == 0) {
        printf("Enter model:");
        gets(model);
        printf("Enter Year:");
        gets(tmp);
        year = atoi(tmp);
        printf("Enter milage:");
        gets(tmp);
        milage = atoi(tmp);
        reg[*pNrOfCars] = createCar(model, year, milage); 
        (*pNrOfCars)++;
        printf("Continue? (yes/no)");
        gets(again);
    }
}

score 1 · Answer 1 · answered Oct 08 '16 at 11:38

1

It's a little bit tricker than it looks. There's not much point just replacing gets with fgets(), if you then process a truncated line on over-long input and handle it as valid. You've merely replaced undefined behaviour with wrong behaviour.

if( fgets(line, sizeof(line), fp) )
{
   if(!strchr(line, '\n'))
   {
      /* line is too long, what you do is up to you, but normally
         we will discard it */
      int ch;

      while( (ch = fgetc(fp)) != EOF)
        if(ch == '\n')
          break;

   }
   else
   {
       /* line is a normal line with a trailing '\n' (gets trims the '\n')           */
   }
}

answered Oct 08 '16 at 11:38

Malcolm McLean

6,258
1
17
18

It's not necessary to just discard long lines ... and long lines occur pretty frequently in text files written by computer programs. `fgets()` can read them ... it just takes more calls, and logic to handle that. – Peter Oct 08 '16 at 11:58
If valid lines are unbounded, fgets() isn't your input function of choice. But of course what an over-long line means is determined by the situation, and sometimes discarding it is wrong. Silently truncating and leaving the remainder indistinguishable from a full line is almost never correct behaviour, however. – Malcolm McLean Oct 08 '16 at 20:02

score 0 · Answer 2 · answered Oct 08 '16 at 08:36

0

Just do for example

if (NULL != fgets(model, WORDLENGTH, stdin)) /* Read the string. */
{
  model[strcspn(model, "\r\n")] = '\0'; /* Cut off \n and/or \r, if any. */
}

answered Oct 08 '16 at 08:36

alk

69,737
10
105
255

I think this is will do it just for model, if i have multiply int's and char, is there a function that can look through all of them and replace the \n? – xxFlashxx Oct 08 '16 at 09:35
@Alex: `fgets()` (as well as`get()`) reads "strings" only. For reading the "year", you use `gets(tmp)`, which can be replaced as shown. – alk Oct 08 '16 at 09:40
so i insert the if statement right after fgets(model)? – xxFlashxx Oct 08 '16 at 10:46
Yes. And you want to add some error handling for the else-case. – alk Oct 08 '16 at 11:21
Also reading the documentation for the functions used I also recommend. – alk Oct 08 '16 at 11:23

chqrlie · Accepted Answer · 2016-10-08T12:02:13.830

You can write a utility function mygets() that takes 2 arguments: a pointer to the destination array and its size:

char *mygets(char *dest, size_t size) {
    /* read a line from standard input and strip the linefeed if any */
    if (fgets(dest, size, stdin)) {
        dest[strcspn(dest, "\n")] = '\0');
        return dest;
    }
    return NULL;
}

void regCars(Car reg[], int *pNrOfCars) {
    char model[WORDLENGTH], tmp[WORDLENGTH];
    int year, milage;

    for (;;) {
        printf("Enter model:");
        if (!mygets(model, sizeof mode))
            break;
        printf("Enter year:");
        if (!mygets(tmp, sizeof tmp))
            break;
        year = atoi(tmp);
        printf("Enter milage:");
        if (!mygets(tmp, sizeof tmp))
            break;
        milage = atoi(tmp);
        reg[*pNrOfCars] = createCar(model, year, milage); 
        (*pNrOfCars)++;
        printf("Continue? (yes/no)");
        if (!mygets(tmp, sizeof(tmp))
            break;
        if (strcmp(again, "yes") != 0)
            break;
    }
}

Note that you can factorize more code with a prompt() function that outputs the question and reads the response:

char *prompt(const char *message, char *dest, size_t size) {
    printf("%s ", message);
    fflush(stdout);
    /* read a line from standard input and strip the linefeed if any */
    if (fgets(dest, size, stdin)) {
        dest[strcspn(dest, "\n")] = '\0');
        return dest;
    }
    return NULL;
}

void regCars(Car reg[], int *pNrOfCars) {
    char model[WORDLENGTH], tmp[WORDLENGTH];
    int year, milage;

    for (;;) {
        if (!prompt("Enter model:", model, sizeof mode))
            break;
        if (!prompt("Enter year:", tmp, sizeof tmp))
            break;
        year = atoi(tmp);
        if (!prompt("Enter milage:", tmp, sizeof tmp))
            break;
        milage = atoi(tmp);
        reg[*pNrOfCars] = createCar(model, year, milage); 
        (*pNrOfCars)++;
        if (!prompt("Continue? (yes/no)", tmp, sizeof(tmp))
            break;
        if (strcmp(again, "yes") != 0)
            break;
    }
}

Also note that this function should take the size of the reg array to stop prompting for more input when it is full. As currently specified, it has the same shortcoming as gets(), unexpected input will cause undefined behavior.

@Alex: do you mind up-voting the answer you accepted? – chqrlie Oct 10 '16 at 23:52 — chqrlie, Oct 10 '16 at 23:52

Replacing gets() with fgets()

3 Answers3