So basically here I am just seeing whether my previously stored input, that is temp->voterDetails[0].name, has its value restored after another input.
Unfortunately it does not seem to store its value. Could you help me understanding why?
#include <stdio.h>
#include <stdlib.h>
struct Voter {
char *name;
};
struct Block{
// Creating a block containing 5 voter details
struct Voter voterDetails[5];
};
int main() {
int choice;
struct Block *Genesis = (struct Block *)malloc(sizeof(struct Block));
struct Block *temp = Genesis;
struct Block *newnode = (struct Block *)malloc(sizeof(struct Block));
char *nm;
int voteNum = 0;
int blockNumber = 1;
do{
printf("What do you want to do\n1. Cast a vote\n2. Check your vote\n3. Exit\n");
scanf("%d", &choice);
switch(choice){
case 1:
printf("Enter name");
fgets(nm, 30, stdin);
fgets(nm, 30, stdin);
voteNum = 1;
temp->voterDetails[voteNum-1].name = nm;
puts(Genesis->voterDetails[0].name);
break;
}
} while (choice != 3);
return 0;
}
CodePudding user response:
At least these problems:
Uninitialized/assign value
Calling fgets(nm, 30, stdin); yet nm is not yet assigned. UB
CodePudding user response:
Your program has a lot of issues:
nmtemporary pointer (that in your snippet isn't even necessary, but I assume it can be useful in the whole program) is not initialized. It should be allocated, instead- Each element of the array
temp->voterDetails[voteNum].namehas to be allocated as well - Strings are copied using
strcpyfunction - Whenever you perform a
choicethe newline remains in the input buffer, making the firstfgetsreturn immediately with no data. That's what you "solved" callingfgetstwice, but it's not a real solution - Data read with
fgetswill have trailing newline, that will have to be removed from thenamebuffer
Here it is the working code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAX_VOTERS 5
struct Voter {
char *name;
};
struct Block{
// Creating a block containing 5 voter details
struct Voter voterDetails[MAX_VOTERS];
};
void flushInput(void)
{
int c;
while ((c = getchar()) != '\n' && c != EOF) { }
}
int main() {
int choice;
struct Block *Genesis = (struct Block *)malloc(sizeof(struct Block));
struct Block *temp = Genesis;
struct Block *newnode = (struct Block *)malloc(sizeof(struct Block));
char *nm = malloc(30 * sizeof(char)); // nm temporary pointer allocation
int voteNum = 0;
int blockNumber = 1;
do{
printf("What do you want to do\n1. Cast a vote\n2. Check your vote\n3. Exit\n");
scanf("%d", &choice);
flushInput();
switch(choice){
case 1:
printf("Enter name\n"); // Better with a newline
fgets(nm, 30, stdin); // Useless extra fgets removed
nm[strcspn(nm, "\n")] = 0;
temp->voterDetails[voteNum].name = malloc(30*sizeof(char));
strcpy(temp->voterDetails[voteNum].name, nm); // Strings cannot be assigned, but are copied with strcpy
// voteNum incremented later, so '-1' can be avoided. Isn't it more readable?
printf("Name: %s..\n", Genesis->voterDetails[voteNum].name); // printf used instead of puts, in order to append a newline
voteNum = 1;
break;
}
} while (choice != 3 && voteNum < MAX_VOTERS); // The program is safer if you check that votes don't exceed the number of location in your array
free(nm);
/* ... remember to free all the remaining pointers as soon as you don't need them anymore */
return 0;
}
Output:
What do you want to do
1. Cast a vote
2. Check your vote
3. Exit
1
Enter name
Roberto
Name: Roberto
What do you want to do
1. Cast a vote
2. Check your vote
3. Exit
...
Some extra comments:
- Pending data in
stdinis flushed using the custom functionflushInput()(consult this answer for more details) - After flushing correctly the input buffer, the extra
fgetscan be removed - It's always better to make sure that the user doesn't insert data we don't have room for. In this case, since the
voterDetailsarray has 5 elements, we make sure to exit the loop as soon as the fifth name has been inserted - Since the constant '5' is now used several times, it is better to avoid using "magic numbers" in the code. Just define a precompiler constant, instead (
MAX_VOTERS)
CodePudding user response:
You're not allocating any space for your pointers. This doesn't happen "automatically" in C, you have to manually do it. From your fgets argument of 30, it seems you want to accept names with a max of 30 chars. If that's the case, then the easiest thing to do is simply use char arrays instead of pointers:
struct Voter {
char name[30]; // note because of the required NUL terminator, this
// will only fit 29 printable characters
};
and the same for nm
char nm[30] = { 0 }; // initializes the array to 0s, altho fgets
// will always add a NUL terminator
But even with those fixes, you still need to copy the string data entered in nm to your names:
#include <string.h>
...
strcpy(temp->voterDetails[voteNum-1].name, nm);
Or IMHO, there's no point in using the temp nm anyway, just plug temp->voterDetails[voteNum].name directly into fgets. Also note that fgets will add a newline to the array if you don't max out the buffer size. You can search for ways to remove that if you don't want it.
If you want to use pointers instead of arrays, you must malloc space for name and nm as you have for your struct Block pointers.
