Reputation: 7
I'm writing a program so that a user can create a text file that stores a list of their dream cars, garage inventory, any list of vehicles. I use fgets and sscanf to ask the user for the make, then year, then model, then value, then color. But for some reason it skips the user input stage of getting the model and jumps to asking for the value, but still displays the printf of "Enter car model: " above the printf for entering the value. Below is the excerpt from my code containing my fgets/sscanf functions and below that will be an example of the output so you can see what the issue is if (ha, if) my description wasn't very clear.
if (!listappend){
printf("File nonexistent/inaccessible.");
return 1;
}
memset(colorin, 0, sizeof(colorin));
memset(makein, 0, sizeof(makein));
memset(modelin, 0, sizeof(modelin));
memset(yearin, 0, sizeof(yearin));
memset(valuein, 0, sizeof(valuein));
printf("\nEnter car make: ");
fgets(makein, sizeof(makein), stdin);
sscanf(makein, "%[^\n]",makein);
fprintf(listappend, "\n%s", makein);
printf("\nEnter car manufacture year: ");
fgets(yearin, sizeof(yearin), stdin);
sscanf(yearin, "%d", &yearinf);
printf("\nEnter car model: ");
fgets(modelin, sizeof(modelin), stdin);
sscanf(modelin, "%[^\n]",modelin);
fprintf(listappend, "\n%s", modelin);
printf("\nEnter approximate car value: $");
fgets(valuein, sizeof(valuein),stdin);
sscanf(valuein, "\n%f", &valueinf);
fprintf(listappend, "\n%d %'.2f", yearinf, valueinf);
printf("\nEnter car color: ");
fgets(colorin, sizeof(colorin), stdin);
sscanf(colorin, "%[^\n]",colorin);
fprintf(listappend, "\n%s", colorin);
fclose(listappend);
Sample output:
|-|-|-|-|-|-|-|-|-|-|Car-lection List|-|-|-|-|-|-|-|-|-|-|
----------------------------------------------------------
[A]dd New Car [V]iew List
[M]enu [D]eveloper Info
[C]lear List [Q]uit
Option?
A
Enter car make: Honda
Enter car manufacture year: 2000
Enter car model:
Enter approximate car value: $5000
Enter car color: Blue
Option?
V
Entry Year Color Make Model Color Approx. Value
1| 2000 Blue Honda, approximate value: $5,000.00
As you can see, the user should be able to input the car model, but the program immediately asks for the approximate car value, skipping the chance to input the car model information. And here is the cat of car_list.txt, the text file I save the strings and values to to be read:
cat car_list.txt
Honda
2000 5,000.00
Blue
I'm new to C programming so if it's a silly mistake I apologize for wasting your time, but it's been driving me crazy. Thank you in advance for your help!
Upvotes: 0
Views: 722
Reputation: 753930
The calls to sscanf()
are incorrect:
sscanf(makein, "%[^\n]", makein);
This leads to undefined behaviour. The formal POSIX (and C99, C11) specification for sscanf()
is:
int sscanf(const char *restrict s, const char *restrict format, ...);
The restrict
means that there may not be any other parameter in the call to sscanf()
that is an alias for the s
argument (or that overlaps with it). In your code, you have makein
as both the source string and the target string, which violates the restrict
criterion, and therefore leads to undefined behaviour.
The repetitions in the code cry out for the use of a function.
You've not shown us the sizes of the strings you've defined. You've also never checked that fgets()
was successful, nor that sscanf()
was successful. My suspicion is that one of your strings is too short, so that the fgets()
doesn't actually read the whole line, and the 'skipped' reads the last part of the previous line. You can diagnose this by printing out the input data. For example, after each (successful) fgets()
, you might print:
printf("<<%s>>\n", makein);
which will ensure that the program has seen the input you expect it to see — one of the most basic of debugging techniques.
So you're saying I need to setup a new variable for each
sscanf
that repeats the first parameter as the third as well? ex of what I'm trying to say:sscanf(makein, "%[^\n]", &makeins)
rather thansscanf(makein, "%[^\n]", makein)
?
More or less, yes.
There are two issues to address. One is that your original calls to sscanf()
yield undefined behaviour — that's the easy bit. The other part is that you assume that the data on each line fits into the variables — and since you didn't show the sizes, it is hard to know whether that's plausible. It also isn't clear whether all the fields are the same size or not; my presumption would be that they're of different sizes.
There are several ways to tackle this. I'd probably write a function along the lines of:
int read_value(const char *prompt, char *buffer, size_t buflen)
{
char line[4096]; // Big!
buffer[0] = '\0'; // Null terminate output in case of EOF.
printf("%s: ", prompt);
fflush(stdout); // Optional
if (fgets(line, sizeof(line), stdin) == 0)
return EOF;
size_t len = strlen(line);
if (line[len-1] == '\n')
line[--len] = '\0';
if (len > buflen)
len = buflen - 1;
memmove(buffer, line, len);
buffer[len] = '\0';
return len;
}
This assumes that truncation of data is OK, and that leading white space does not need to be skipped, nor trailing white space other than the newline. Removing leading space is trivial: strspn()
is ideal. Removing trailing space is less trivial; you have to write a decrementing loop yourself, taking care to handle an all-blank line correctly.
The function prints the prompt string and adds a colon and blank at the end; if you don't want that, use just printf("%s", prompt);
or fputs(prompt, stdout);
— do not use printf(prompt);
because that crashes horribly if the prompt ever contains a percent symbol: "Enter discount (%)"
, for example. The fflush()
is not usually needed (the system will usually do it for you), but it makes sure the prompt is visible.
The fgets()
reads into a big line buffer; for all practical purposes, it ensures you read the full line. The test for newline could be given an else
clause that handles a truncated line — JSON data (bookmarks files, for instance) can be humungous single lines of data — and you could report an error, or gobble to the end of line or EOF, or whatever other mechanism seems appropriate to you.
The code also truncates the data that was read if it won't fit in the buffer passed to the function — again, you could report an error if you preferred, or use a more sophisticated truncating algorithm than simply overwriting the data at the maximum length (search for the prior word break, for example).
The memmove()
plus assignment null terminates the data after the copy. The length of the string is returned (or EOF on error). This is something that a better version of fgets()
would do automatically (and the POSIX getline()
function already does).
Using this function, your code that reads:
printf("\nEnter car make: ");
fgets(makein, sizeof(makein), stdin);
sscanf(makein, "%[^\n]",makein);
would become:
if (read_value("\nEnter car make", makein, sizeof(makein)) == EOF)
…handle error…
Your existing code ignores EOF, so this is an improvement. I could also note that your memset()
calls are not really necessary; the strings returned will be null terminated.
Upvotes: 2