CODEWITHSUNDEEP
c
Alphonse
Alphonse

Reputation: 671

C checking file for string from scanf

So I give the user the option to introduce a code for a car like this:

typedef struct
{
    char name[50];
    char category[50];
    int code;
    int price;
} car;
car c;
    FILE *f;
    f = fopen("Cars.txt", "r+");
    printf("Enter the code of the car:");
    if (f != NULL) {
        scanf("%d", &c.code);
        char *code;
        code = (char *)malloc(sizeof(c.code) +1 );
        sprintf(code, "%d", &c.code);
        while (checkCode(code, f) == 0)
        {
            printf("The code for this car already exists, try again:");
            scanf("%d", &c.code);
            char *code;
            code = (char *)malloc(sizeof(c.code) + 1);
            sprintf(code, "%d", &c.code);
        }
    }

And this is my checkCode function:

int checkCode(char *code, FILE *f) {
    char line[1024];
    while (fgets(line, sizeof(line), f) != NULL)
    {
        if (strstr(line, code) != NULL)
        {
            return 0;
        }
    }
    return 1;
}

It doesn't work. Even though I introduce a code already entered on one of the file's lines the function returns 1. My guess is the problem is here:

while (fgets(line, sizeof(line), f) != NULL)

I am new to C. Can anyone please explain what problem I have. Thank you all for your time!

Upvotes: 2

Views: 552

Answers (3)

ilkkachu
ilkkachu

Reputation: 6537

I don't really like the fact that you have the prompt for the code in two places in the code. If you want to go back to beginning when the user gives an unacceptable code, do just that. That might also avoid doing the memory allocation in two places, making it less likely to forget to free one of the buffers. A mock-up:

do {
    int code = ask_for_code();
    if (look_for_code(code) == 0) {
        printf("sorry, that code already exists");         
    } else {
        do_something_with_the_code();
        break;
    }
} while(1);

Then ask_for_code() would contain your prompt and scanf, and look_for_code approximately matches your checkCode().

Change the loop condition to a status flag or something like that if you don't like seemingly neverending loops.

Also this:

code = (char *)malloc(sizeof(c.code) +1 );
sprintf(code, "%d", &c.code);

allocates a buffer the size of an int (+1), so probably about 5 bytes. That's enough for a number up to 9999, but anything past that will overflow the buffer. A 32-bit integer should fit in 12 bytes (with the tailing zero), but in any case, you might want to use snprintf there:

char code[12];
snprintf(code, 12, "%d", c.code);

That printf should also take the number itself (c.code), not its address (&c.code).

Upvotes: 1

chqrlie
chqrlie

Reputation: 145297

There are several problems in your code:

  • you allocate space for the converted string with an inconsistent size: the number of characters for the number conversion cannot be computed as sizeof(c.code) + 1, you should instead use a local array with a fixed size.
  • you pass the address of the code instead of its value.
  • you do not rewind the stream pointer to scan the file from the beginning.
  • you should also seek to the end of the stream to write the new entry.

Here is a corrected version:

FILE *f;
f = fopen("Cars.txt", "r+");
if (f != NULL) {
    char code[32];
    printf("Enter the code of the car:");
    scanf("%d", &c.code);
    snprintf(code, sizeof code, "%d", c.code);
    rewind(f);
    while (checkCode(code, f) == 0) {
        printf("The code for this car already exists, try again:");
        scanf("%d", &c.code);
        snprintf(code, sizeof code, "%d", c.code);
        rewind(f);
    }
    fseek(f, 0L, SEEK_END);
}

It would actually be more reliable to read the response as a string and convert with sscanf():

FILE *f = fopen("Cars.txt", "r+");
if (f != NULL) {
    char code[32];
    printf("Enter the code of the car:");
    for (;;) {
        if (fgets(code, sizeof code, stdin) == NULL) {
            printf("unexpected end of file\n");
            return -1;
        }
        if (sscanf(code, "%d", &c.code) != 1) {
            printf("invalid input, try again:");
            continue;
        }
        snprintf(code, sizeof code, "%d", c.code);
        rewind(f);
        if (checkCode(code, f) == 0) {
            printf("The code %s is already used, try again:", code);
            continue;
        }
        break;  /* the code in c.code is OK */
    }
    fseek(f, 0L, SEEK_END);
}

Upvotes: 3

slingeraap
slingeraap

Reputation: 598

One problem I see is that the file is not rewound after one interation of the while loop.

On the second time the check is run, the file pointer will still be at the end of the file, so it will act as if there are no codes in the file.

Another tip:

use do...while here, since you want to ask for input at least once. This will reduce code duplication. Something like this:

char code[256];

do
{
   scanf("%d", &c.code);
   sprintf(code, "%d", &c.code);
} while (checkCode(code, f) == 0);

(Note that the error message for wrong input should be moved to the checkCode() function)

Upvotes: 1

Related Questions