struct sdata;
typedef struct sdata *sptr;
struct sdata {
char sName[25];
char sNum[7];
sptr next;
};
void readdata();
int main(void)
{
readdata();
return 0;'
}
void readdata(){
int length;
int i ;
char Line [100];
const char s[2] ="#";
char *token= strtok(Line,s);
FILE * data =fopen("data.txt","r") ;
if(data==NULL){
printf("ERROR : The File Cannot Be Found");
}else{
/*Read The Student File Line By Line */
while(fgets(Line,100,Student_info)!=NULL ){
printf("-----> %s\n",Line);
sptr stemp = (sptr)malloc(sizeof(struct data));
stemp->next=NULL;
token = strtok(Line,s);
strcpy(stemp->sName, token);
token = strtok(NULL,s);
strcpy(stemp->sNum, token);
while(token !=NULL){
token = strtok(NULL,s);
length=strlen(token);
if(token[length-1]=='\n')
token[length-1]='\0';
}
stemp=stemp->next;
}
}
} Mrra 0 Newbie Poster
Dani AI
Generated
Good troubleshooting so far — fixed most of the bugs that stop a loop from reading past the first line. The single biggest root cause in the original post by is calling parsing routines on an uninitialized buffer (and then using that same buffer incorrectly). A robust read/parse loop and defensive checks are what prevent the program from stopping after one line.
Use the fgets loop idiom (not feof) so each iteration begins with a valid line:
while (fgets(Line, sizeof Line, Student_Info) != NULL) {
/* trim trailing newline, then parse the just-read Line */
} Parse safely: strtok modifies the buffer and can return NULL — check each token before using it and never do strlen(token) unless token is non-NULL. Prefer strtok_r (or manual scanning/sscanf) if the code will be reused or multithreaded. When copying into fixed-size fields, use snprintf(..., sizeof field, "%s", token) or strncpy followed by an explicit field[sizeof field - 1] = '\0' to avoid overflow.
Memory and bookkeeping: always check malloc's result, link new nodes into the list via a root/tail (or insert-at-head) pointer instead of doing stemp = stemp->next; (which leaks the node). Close the file and free the list when finished. Compile with -Wall -Wextra and run under Valgrind or a sanitizer to catch leaks and invalid reads/writes.
Quick checklist:
- print each Line right after fgets to confirm the file is being read,
- trim CR/LF and strip BOM if the file came from Windows/Excel,
- check every pointer returned by
strtok/mallocbefore use, - ensure your buffer sizes include space for the terminating NUL.
Following these steps will make the reader loop reliable and avoid the “only one line” symptom.
Nutster 58 Newbie Poster
There were several problems with your code. I had to make some significant changes just to get it to compile.
- I added
#includelines at the top to read in the required function declarations. - You had two different names for your file handle.
Student_Infowas a better name than justdata, so I kept that one and changeddatato match. - You used the wrong name in
malloc. The inner argument needed to bestruct sdata; you wrotestruct data, which was not defined. - I removed extranous characters. (e.g. See the end of your line 15)
I refactored your code, making the following adjustements to make it easier to read, maintain and less vulnerable to problems.
- I added blank lines to make things easier to read. You generally want to put a blank line after your variable declarations, before the rest of your function body.
- I removed some blank lines that were visually breaking up the flow of your program, such as the blank line between the data members of your struct and the pointer to the next node.
- I adjusted the indentation of the code to make it easier to read. Well, my IDE did most of the work, but I helped. The curly braces around blocks of code should start on their own line.
- I moved the
mainfunction to the end, so that the function definition forreaddatacould also act as the declaration; you did not need a seperate declaration line for it. - The
readdatafunction looks like it is supposed to read the data then do something with it. You have it returning nothing:void. I changed it to return the linked list it was building. More about that as we go on. - I decided to pass the filename to the
readdatafunction to increase flexibility. I also checked to make sure it was not a NULL pointer as one of the first actions I took in the function. - I removed the initialization for
tokenas it did not make sense there as Line did not have anything in it yet. - I removed
const char s[2] = "#";. Normally I like putting constants before the variables, but that was not main problem. This constant does not add to readability later on. Maybe a better name would help, but in this case, I decided to just put the string in the function calls. - I moved the declaration of
stempout of the loop. There is no need to recreate the variable each time through the loop. - I added
sRootto point to the first node of the linked list. I addedsAddingto keep track of where the next element of the linked list gets added. - I removed
ifrom the declaration list as it is not used in the function. - If the
fopencall fails, then I not only print the error message, but also return an empty linked list. I added the filename to the error message. - I checked the end-of-file as the loop condition and moved the
fgetscall inside the loop. If thefgetscall fails, it still gets out of the loop by usingbreak;. - I simplified the line output message by removing the leading arrow.
- Always make sure the return value from
mallocis not NULL. If you run out of memory, your program has just begun to fail. In that circumstance, I close the file and return the linked list that has already been read. - Always make sure that a pointer is not null before following it with
*or->. - I changed the delimiter to
strtokto "#\n". This way, not only doesstrtoklook for the hash marks between fields, but also the new line at the end of the string. This also makes it easier to manage malformed lines by looking for new line at the end of the read string. - Check if
strtokreturned NULL, meaning it could not find the given delimiters. In that case, you have a bad text line. Ifreethe pointer to prevent a memory leak and restart the loop. - If
strtokreturns a non-NULL pointer, it will copy the string into the node. I have usedstrncpyinstead ofstrcpyto prevent string overflow. - If
strncpyreaches the limit of characters, it does not put in a null character at the end of the string, so I added a line to put a null characters at the end of the array. If the Name is shorter than 25 characters, putting a null character after the shorter string will do nothing to it. If the Name is 26 characters or longer, it will put the null character at the end of the string, just a C expects. - The loop at the end is not needed. You do not have to go through the rest of the string to get ready to read the next line. When you give
strtoka new string, it resets, completely forgetting about the last string. Also, I removedlengthfrom the variable declarations as it no longer needed. - I am not sure what
stemp = stemp->next;is supposed to do, but what it is doing is creating a memory leak by throwing away the the node you just created without using thefreefunction. I have added code to actually add the new node to the linked list instead. - At the end, I return the linked list so that
maincan do something with it, in this case printing the data that was successfully read by walking through the linked list.
My modified code:
/* these are needed to declare the appropriate functions */
#include <stdio.h> /* fopen, fgets, fclose, printf */
#include <string.h> /* strtok */
#include <stdlib.h> /* malloc, free, NULL */
struct sdata;
typedef struct sdata *sptr;
struct sdata {
char sName[25];
char sNum[7];
sptr next;
};
sptr readdata(char *filename) /* Return the linked list you built. Passing the filename increases flexibility. */
{
char Line[100];
char *token; /* Initialization was premature */
FILE *Student_Info; /* use a consistent variable name */
sptr stemp; /* no need to recreate this every time through the loop */
sptr sRoot = NULL; /* the start of the linked list. Starts with nothing in it. */
sptr sAdding = NULL; /* The last node in the linked list. This will be where new nodes get added to the list. */
/* make sure the filename was given properly */
if (filename == NULL)
return NULL; /* nothing to do, so go back to caller */
Student_Info = fopen(filename, "r");
if (Student_Info == NULL)
{
printf("ERROR: The file %s cannot be found.", filename);
return NULL;
}
/* Read The Student File Line By Line */
/* Check for end-of-file at the start of the loop. */
while (!feof(Student_Info))
{
if (fgets(Line, 100, Student_Info) == NULL)
/* if we do not get a valid line, break the loop */
break;
printf("%s\n", Line);
stemp = (sptr)malloc(sizeof(struct sdata)); /* used name of structure, not FILE */
if (stemp == NULL) /* Added check: If malloc fails (out of memory), abort */
{
fclose(Student_Info);
return sRoot;
}
stemp->next = NULL;
/* Look for a # or a new line. */
token = strtok(Line, "#\n");
if (token == NULL) /* Added check: if it can not find the token */
{
/* release the memory for the linked list node */
free(stemp);
/* go back and try reading the next line */
continue;
}
else
{
/* I am using strNcpy to control how many characters to copy.*/
strncpy(stemp->sName, token, 25);
/* put a null character at the end of the string, just in case we read too long a string. In that case, strncpy does not append the null at the end of the string. */
stemp->sName[24] = '\0';
}
token = strtok(NULL, "#\n");
if (token == NULL) /* Check to see if there is no second token. In that case, free this structure, abandon the line and try again. */
{
/* release the memory for the linked list node */
free(stemp);
/* go back and try reading the next line */
continue;
}
else
{
/* I am using strNcpy to control how many characters to copy.*/
strncpy(stemp->sNum, token, 7);
/* put a null character at the end of the string, just in case we read too long a string. In that case, strncpy does not append the null at the end of the string. */
stemp->sNum[6] = '\0';
}
/* Add to linked list */
if (sRoot == NULL) {
/* if the root of the linked list is NULL, start the linked list with the current node. */
sRoot = stemp;
sAdding = sRoot;
}
else
{
/* Otherwise, add the current node at the end of the linked list. */
sAdding->next = stemp;
/* Move the adding point to the new node */
sAdding = stemp;
}
}
/* close the file when done reading. */
fclose(Student_Info);
/* Return the linked list that was just read. */
return sRoot;
}
int main(void) /* relocated to reduce clutter */
{
sptr sList;
sptr sWalker;
sList=readdata("data.txt");
/* do what you want with list. */
/* like, print it!*/
for (sWalker = sList; sWalker != NULL; sWalker = sWalker->next)
printf("%s: %s\n", sWalker->sNum, sWalker->sName);
return 0;
} Edited by Nutster because: Fix wording.
Be a part of the DaniWeb community
We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.