Im using a double linked list to make a studentdata base, however im having trouble trying to get all my scanfs and fgets to actually work, after scanf(lastname) is declared it freezes, instead of going on to next printf statement asking for the classification. I added my .h file code therefore if oyu could let me know i made a error. I tried using a function to clear the stdin
//.h file
#ifndef STUDENT_H_
#define STUDENT_H_
typedef struct student{
char* name;
char* lastname;
long studentID;
char* class;
int grad_Year;
struct student *next;
struct student *prev;
}student_t;
typedef struct DLL{
//struct student *next;
//struct student *prev;
struct student* head; //pointer to head node
struct student* tail; //pointer to tail
}DLL_t;
#endif /* STUDENT_H_ */
#ifndef STUDENT_C_
#define STUDENT_C_
#include<stdio.h>
#include<stdlib.h>
#include <string.h>
#include "Student.h"
#define BUFFERSIZE 128
//typedef struct student{
//
// char* name;
// char* lastname;
// long studentID;
// char* class;
// int grad_Year;
//
//};
//Global Var
typedef struct student studentNode; //creates new struct studentNode with student attrigutes
typedef studentNode *StudentPtr; //creates studentptr
typedef struct DLL dllNext;
typedef dllNext *next;
//Prototypes
void instructions(void){
printf( "Enter your choice:\n"
" 1 to insert an element into the list.\n"
" 2 to delete an element from the list.\n"
" 3 to end.\n" );
}
void insert(studentNode **headList,char* name, char* lastname, char* class, long studentID,int grad_Year){
//starters
StudentPtr newPtr;
StudentPtr prevPtr;
StudentPtr currentPtr;
newPtr = ( studentNode * ) malloc( sizeof( studentNode ) );
if(newPtr != NULL){
newPtr->name = name;
newPtr->lastname = lastname;
newPtr->class = class;
newPtr->studentID = studentID;
newPtr->grad_Year = grad_Year;
newPtr->next = NULL;
prevPtr = NULL;
currentPtr = *headList;
if(currentPtr != NULL){
prevPtr = currentPtr;
currentPtr=currentPtr->next;
}
if(prevPtr == NULL){
newPtr->next = *headList;
*headList = newPtr;
}
else{
prevPtr->next = newPtr;
newPtr->next = currentPtr;
}
}
}
void delete(){
printf("In delete function");
}
void clean_file_buffer(FILE *fp)
{
int c;
while((c = fgetc(fp)) != '\n' && c!=EOF);
}
int main(int argc, char *argv[]){
int option;
StudentPtr newPtr;
StudentPtr head = NULL;
instructions();
printf("your option: ",&option);
scanf("%d",&option);
while(option != 3){
switch(option){
case 1:
//insert(&name,&lastname,&class);
//printf("insert\n");
printf("Enter first name: ");
fgets(newPtr->name,BUFFERSIZE,stdin);
newPtr->name[strcspn (newPtr->name, "\n")] = 0;
//scanf("%s",newPtr->name);
//fgets(newPtr->name,BUFFERSIZE,stdin);
//printf("Name is %s\n", newPtr->name);
printf("Enter last name: ");
fgets(newPtr->lastname,BUFFERSIZE,stdin);
newPtr->lastname[strcspn (newPtr->lastname, "\n")] = 0;
//scanf("%s",newPtr->lastname);
//printf("Last name is\n %s",newPtr->lastname);
printf("Enter class: ");
scanf("%s",newPtr->class);
//fgets(newPtr->class,BUFFERSIZE,stdin);
// printf("Enter StudentID");
// scanf("%ld",newPtr->studentID);
//
// printf("Enter Graduation Year");
// scanf("%d", newPtr->grad_Year);
insert(&head,newPtr->name,newPtr->lastname, newPtr->class, newPtr->studentID, newPtr->grad_Year);
break;
default:
printf("Invalid choice.\n\n");
instructions();
break;
}
}
}
#endif /* STUDENT_C_ */
CodePudding user response:
Okay, It's a bit difficult to know where to start -- you were obviously a bit lost. Let's start with a couple of tips:
- You need to review Is it a good idea to typedef pointers?. (hint: Answer - No)
- There is no need to guard your C source against multiple inclusion with
#ifndef STUDENT_C_
, but Good job guarding your header against multiple inclusion with#ifndef STUDENT_H_
. - Types ending in
_t
are reserved by POSIX (though it's unlikely they will have astudent_t
need, butDLL_t
is less clear). This is a rule to keep in mind, though I find myself in violation of it every once in a while:)
- In C, there is no need to cast the return of
malloc
, it is unnecessary. See: Do I cast the result of malloc? - This is taste, but the
'*'
when declaring a pointer goes with the variable and not the type. Why?
char* a, b, c;
Does NOT declare 3 pointers to char
. It declares one pointer to char
(a
) and two character variables (b
& c
). Writing
char *a, b, b;
Makes that clear.
Other than the problem you had with fgetc(fp)
blocking when there was no input after fgets()
-- detailed in my comments below your question, your biggest problem was failing to allocate for name
, lastname
and class
members of your student_t
struct. When you attempted to fill newPtr->name
, etc.. your program croaked (invoked Undefined Behavior) by attempting to write to memory that you didn't allocate. (a SegFault was possible).
Every time you allocate, you must validate the allocation. It's not "if" malloc()
fails, but "when" you are protecting against. Also, there is no need to take input for studentID
. Just declare a long
(or better unsigned long
) as a member of DLL_t
and then assign and increment it each time a student is validly entered.
There are a few other things noted in the comments below, but really it was just cleaning up your insert()
and breaking it into two functions a createnode()
function that fully allocates and initializes the struct values and leaving insert()
to just handle the list operations. Print and delete functions were added to allow you to print the current list and then clean-up by freeing all allocated memory before program exit.
Your header becomes:
#ifndef STUDENT_H_
#define STUDENT_H_
typedef struct student{
char *name;
char *lastname;
long studentID;
char *class;
int grad_Year;
struct student *next;
struct student *prev;
}student_t;
typedef struct DLL{
struct student *head; //pointer to head node
struct student *tail; //pointer to tail
long maxID; /* temporarily used to assign studentID */
} DLL_t;
#endif /* STUDENT_H_ */
Your cleaned up program then becomes:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "student.h"
/* BUFFERSIZE shortened because I don't like typing... */
#define MAXC 128
// Definitions
void instructions(void){
/* align it so it looks like your menu */
fputs ( "\nEnter your choice:\n"
" 1. to insert an element into the list.\n"
" 2. to delete an element from the list.\n"
" 3. view list.\n"
" 4. to end.\n\n"
" Your choice: ", stdout);
fflush (stdout); /* only needed when not followed by input function */
}
/** simple implementation of POSIX strdup() */
char *strdupe (const char *src)
{
char *dst = NULL;
size_t len = strlen (src); /* get length of source */
if (!(dst = malloc (len 1))) { /* allocate/validate destination */
perror ("malloc-strdupe");
return NULL;
}
return memcpy (dst, src, len 1); /* return pointner to dst */
}
/** create node function, allocates and validates the node and
* each member of student_t requring allocation and set the member
* values to the value provided in the parameters, initializing the
* next/prev pointers NULL. Returns pointer to allocated node on success
* NULL otherwise.
*/
student_t *createnode (char *name, char *lastname, char *class,
long studentID, int grad_Year)
{
student_t *node = malloc (sizeof *node); /* allocate for node */
if (!node) { /* validate allocation for node */
perror ("malloc-createnode-dst");
return NULL;
}
node->next = node->prev = NULL; /* initialize node pointers null */
if (!(node->name = strdupe (name))) { /* allocate/validate name */
return NULL;
}
if (!(node->lastname = strdupe (lastname))) { /* ditto - lastname */
return NULL;
}
if (!(node->class = strdupe (class))) { /* ditto - class */
return NULL;
}
/* assign remaining member values */
node->studentID = studentID;
node->grad_Year = grad_Year;
return node; /* return pointer to allocated node */
}
/** must have return type to indicate success/failure of insert.
* you cannot just blindly assume allocations succeed. It's not
* "if", but "when" an allocation fails.
*
* returns pointer to allocated node on success, NULL on failure.
*/
student_t *insert (DLL_t *list, char *name, char *lastname,
char *class, long studentID, int grad_Year)
{
/* create and validate new node */
student_t *node = createnode (name, lastname, class,
studentID, grad_Year);
if (!node) {
return NULL;
}
if (!list->head) /* if 1st node, node is head/tail */
list->head = list->tail = node;
else { /* otherwise */
node->prev = list->tail; /* set prev to tail */
list->tail->next = node; /* add at end, update tail pointer */
list->tail = node;
}
return node; /* return new node */
}
/** print helper for single node */
void prn_node (student_t *node)
{
printf ("\nStudentID : %ld\n"
"First name : %s\n"
"Last name : %s\n"
"Class : %s\n"
"Graduates : %d\n",
node->studentID, node->name, node->lastname,
node->class, node->grad_Year);
}
/** print all nodes in list */
void prn_list (DLL_t *list)
{
if (!list->head) {
puts ("list-empty");
return;
}
/* loop over each node in list */
for (student_t *n = list->head; n; n = n->next) {
prn_node (n);
}
}
/** delete single node helper */
void del_node (student_t * node)
{
free (node->name);
free (node->lastname);
free (node->class);
free (node);
}
/** delete all nodes in list */
void del_list (DLL_t *list)
{
student_t *n = list->head;
while (n) {
student_t *victim = n; /* create temporary to delete */
n = n->next; /* advance node pointer to next */
del_node (victim); /* now free current node */
}
list->head = list->tail = NULL;
list->maxID = 0;
}
int main (void) { /* argument is void if you pass no arguments */
char buf[MAXC]; /* tmp buffer for all input */
int option;
DLL_t list = { .head = NULL}; /* initialize DLL_t struct pointers
* no need to allocate.
*/
while (1) { /* loop continually - use case to exit */
instructions(); /* show prompt */
if (!fgets (buf, MAXC, stdin)) { /* validate input received */
puts ("(user canceled input)");
return 0;
}
if (sscanf (buf, "%d", &option) != 1) { /* parse int from buf */
fputs ("error: invalid integer input.\n", stderr);
return 1;
}
switch (option) {
case 1:
{
/* temp storage for strings */
char name[MAXC], lastname[MAXC], class[MAXC];
int grad_Year = 0;
/* there is no conversion in the string - no need for printf() */
fputs ("\nEnter first name : ", stdout);
if (!fgets (name, MAXC, stdin)) {
puts ("(user canceled input)");
return 0;
}
name[strcspn (name, "\n")] = 0; /* trim \n from name */
fputs ("Enter lastname : ", stdout);
if (!fgets (lastname, MAXC, stdin)) {
puts ("(user canceled input)");
return 0;
}
lastname[strcspn (lastname, "\n")] = 0; /* trim \n from lastname */
fputs ("Enter class name : ", stdout);
if (!fgets (class, MAXC, stdin)) {
puts ("(user canceled input)");
return 0;
}
class[strcspn (class, "\n")] = 0; /* trim \n from class */
fputs ("Enter grad year : ", stdout);
if (!fgets (buf, MAXC, stdin)) {
puts ("(user canceled input)");
return 0;
}
if (sscanf (buf, "%d", &grad_Year) != 1) { /* parse grad_Year */
fputs ("error: invalid integer input.\n", stderr);
return 1;
}
/* validate insertion of values into list */
if (!insert (&list, name, lastname, class, list.maxID, grad_Year)) {
continue;
}
list.maxID = 1; /* increment maxID on success */
break;
}
case 2: /* fallthhrough to 3 intentional until something to do */
case 3:
prn_list (&list);
break;
case 4:
goto donelooping;
break;
default:
/* no conversions, printf() no needed, use puts */
puts ("Invalid choice.");
break;
}
}
donelooping:;
del_list (&list); /* free all allocated memmory */
}
(note: POSIX provides the strdup()
function that allows you to allocate for and copy any string. Not knowing if that is available to you, the strdupe()
function in the code above does the same thing. Up to you which one you want to use if you have strdup()
available.)
Always compile with warnings enabled, and do not accept code until it compiles without warning. To enable warnings add -Wall -Wextra -pedantic
to your gcc/clang
compile string (also consider adding -Wshadow
to warn on shadowed variables). For VS (cl.exe
on windows), use /W3
. All other compilers will have similar options. Read and understand each warning -- then go fix it. The warnings will identify any problems, and the exact line on which they occur. You can learn a lot by listening to what your compiler is telling you.
The code above compiles without a single warning. The compile string used was:
$ gcc -Wall -Wextra -pedantic -Wshadow -std=c11 -Ofast -o bin/student student.c
Example Use/Output
A quick run of the program provides:
$ ./bin/student
Enter your choice:
1. to insert an element into the list.
2. to delete an element from the list.
3. view list.
4. to end.
Your choice: 1
Enter first name : Mickey
Enter lastname : Mouse
Enter class name : Disney
Enter grad year : 1911
Enter your choice:
1. to insert an element into the list.
2. to delete an element from the list.
3. view list.
4. to end.
Your choice: 1
Enter first name : Minnie
Enter lastname : Mouse
Enter class name : Disney3
Enter grad year : 1913
Enter your choice:
1. to insert an element into the list.
2. to delete an element from the list.
3. view list.
4. to end.
Your choice: 1
Enter first name : Pluto
Enter lastname : (the dog)
Enter class name : BowWow
Enter grad year : 1922
Enter your choice:
1. to insert an element into the list.
2. to delete an element from the list.
3. view list.
4. to end.
Your choice: 2
StudentID : 0
First name : Mickey
Last name : Mouse
Class : Disney
Graduates : 1911
StudentID : 1
First name : Minnie
Last name : Mouse
Class : Disney3
Graduates : 1913
StudentID : 2
First name : Pluto
Last name : (the dog)
Class : BowWow
Graduates : 1922
Enter your choice:
1. to insert an element into the list.
2. to delete an element from the list.
3. view list.
4. to end.
Your choice: 4
Memory Use/Error Check
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind
is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ valgrind ./bin/student
==4728== Memcheck, a memory error detector
==4728== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4728== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==4728== Command: ./bin/student
==4728==
Enter your choice:
1. to insert an element into the list.
2. to delete an element from the list.
3. view list.
4. to end.
Your choice: 1
Enter first name : Mickey
Enter lastname : Mouse
Enter class name : Disney
Enter grad year : 1911
Enter your choice:
1. to insert an element into the list.
2. to delete an element from the list.
3. view list.
4. to end.
Your choice: 4
==4728==
==4728== HEAP SUMMARY:
==4728== in use at exit: 0 bytes in 0 blocks
==4728== total heap usage: 6 allocs, 6 frees, 2,124 bytes allocated
==4728==
==4728== All heap blocks were freed -- no leaks are possible
==4728==
==4728== For lists of detected and suppressed errors, rerun with: -s
==4728== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
(but note, beginning with version 3.13
valgrind no longer mask system memory allocations, so memory allocated by stdio
(fopen()
, etc..) will remain shown as in-use, generally around 1024 bytes. You must still fclose()
each file stream opened, or more memory will be shown as in-use)
Look things over, and read the comments. Let me know if you have further questions.
CodePudding user response:
At first: this part of your code
int option;
StudentPtr newPtr;
StudentPtr head = NULL;
instructions();
printf("your option: ", &option);
scanf("%d", &option);
while (option != 3)
{ //...
has problems:
dll.c(96,29): warning C4474: 'printf' : too many arguments passed for format string
dll.c(96,29): message : placeholders and their parameters expect 0 variadic arguments, \
but 1 were provided
dll.c(108): error C4700: uninitialized local variable 'newPtr' used
- initialize all variables. Here for example
option
andnewPtr
are not. Even if your compiler tolerates it. But in this caseoption
has an unknown value and you try to use it as condition in a loop. - Never
typedef
pointers. Is is a shot in your own foot. Even if you use aPtr
suffix, or a P or whatever.C
has it covered since the beginning: use an asterisk. It is known by everybody, compiler included, that ifStudent
is astruct
Student*
is a pointer to it. AndStudent**
is a pointer to a pointer to it, and so on.
typedef struct student
studentNode; // creates new struct studentNode with
// student attrigutes
typedef studentNode* StudentPtr; // creates studentptr
typedef struct DLL dllNext;
typedef dllNext* next;
How would someone know, even you tomorrow, that next
is a pointer to dllNext
?
// Global Var
About this comment and the following lines in your code: Do not use globals. Never.
Scanf freezes after second inputs
About your question: scanf()
does not freeze at all. The "problem" is that scanf()
does not consume the \n
that ends the input. And the system can not decide that this \n
is garbage or should be flushed since no one can be sure it is. It is up do the program logic. And can be tons of things typed by the program user and not read yet...
looking for scanf()
behaviour: an example
Writing programs are a certified way to learn to program. But also read the manuals. scanf()
returns an int
. ALWAYS test it. scanf()
is a scanner. A very smart one. scan formatted input is the objective of scanf
, that is the origin of the name. It is great for consuming tabular data, things like csv
files. keyboard is not formatted input, so when you scanf
on stdin
you must be prepared...
And how to be prepared?
an example
#include <stdio.h>
#include <string.h>
int main(void)
{
char buffer[30] = {0};
int res = scanf("s", buffer);
printf("scanf() returned %d. size of string read is %d\n",
res, strlen(buffer));
if ( res != 1 ) return -1;
printf(
"now trying to read a char. Would block the "
"program?\n");
int c = fgetc(stdin);
printf("Read something. Code of char is %d\n", c);
return 0;
}
This program tries to read a 10-char string. If scanf()
is able to read something it returns 1
since the call was for scan for exactly ONE string
.
Here is the output when someone enters "12345" and press ENTER:
12345
scanf() returned 1. size of string read is 5
now trying to read a char. Would block the program?
Read something. Code of char is 10
As you see it by the prompt "now trying..." the program goes on and the call to fgetc()
read a char
with value 10
a.k.a the \n
that ended the call to scanf
.
This is the way it works.
Look at the output here for a 2nd run of the program:
1234512345Agfkdjghkfdghfkdgdkfhgfdkjghfd
scanf() returned 1. size of string read is 10
now trying to read a char. Would block the program?
Read something. Code of char is 65
scanf()
returns as soon as it reads the 10 asked-for char'. The
fgetc()takes the
A,
65in
ASCII`. But the rest stays there to be read.
A note about your code
[a bit off-topic here]
a linked list is a list of nodes
. It is not a list of student
or anything. Each linked node points to or contains some data. And in your program it is the Student
.
- Do not mix the
List
with the data. Never. Why? This is the way to write the code for linked list only once. - for the same reason do not put
main
function inside the same file that contains the implementation of the list
an example of yor header, changed
#ifndef student_h_
#define student_h_
typedef struct
{
char* name;
char* lastname;
long id;
char* class;
int grad_year;
} Student;
typedef struct st_node
{
struct st_node* next;
struct st_node* prev;
Student S;
//Student* S; could be better
} Node;
typedef struct
{
size_t size;
node* head;
node* tail;
} List;
#endif /* student_h_ */
Here you see
- that each
List
has a size, and aList
can for sure be empty. List
does not mentionStudent
at all
from your code
void insert(
studentNode** headList, char* name, char* lastname,
char* class, long studentID, int grad_Year)
StudentPtr newPtr;
StudentPtr prevPtr;
StudentPtr currentPtr;
// ...
You want to insert a Student
in the List
. The ideal way should be generic, but see this one
int insert(Student* s, List* l);
This is what you want:
- No need for the 3 (!) pointers
- absolutely no need for each field as an argument for
insert
. This is encapsulation. The way you did if you add a field toStudent
you will need to change everyinsert
call. - do not return
void
. Return status, return the size of the list, anything.void
is a waste.
I will not provide a full implementation since it is a bit off-topic but I can provide one case you need. Just comment below.