xxFlashxx
xxFlashxx

Reputation: 271

Replacing gets() with fgets()

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);
    }
}

Upvotes: 7

Views: 2129

Answers (3)

chqrlie
chqrlie

Reputation: 144715

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.

Upvotes: 0

Malcolm McLean
Malcolm McLean

Reputation: 6404

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')           */
   }
}

Upvotes: 1

alk
alk

Reputation: 70931

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. */
}

Upvotes: 0

Related Questions