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:
nm
temporary 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].name
has to be allocated as well - Strings are copied using
strcpy
function - Whenever you perform a
choice
the newline remains in the input buffer, making the firstfgets
return immediately with no data. That's what you "solved" callingfgets
twice, but it's not a real solution - Data read with
fgets
will have trailing newline, that will have to be removed from thename
buffer
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
stdin
is flushed using the custom functionflushInput()
(consult this answer for more details) - After flushing correctly the input buffer, the extra
fgets
can 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
voterDetails
array 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 name
s:
#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.