You have a number of issues, not the least of which is the lack of a Minimal Complete Reproducible Example, but your primary problem is you are thinking about reading and parsing the information from the file in an overly complicated and very fragile way.
When you look at your input file, e.g.
cat dat/txtscore.txt
// txt
3 // this int num
1 jay 100 90 80 270 90.0
2 key 90 80 65 235 78.3
3 ray 90 45 65 200 66.7
(empty line at end intentional)
You don't care about the 3
telling you how many records there are -- really. You don't care about the comment lines or the empty line at the end. All you care about are the lines holding the data you want. To that end, all you really care about are the lines that will parse successfully given your input routine.
No matter what you are reading, a good general approach is to read an entire line of data into a buffer (temporary character array) with fgets()
and then attempt to parse the line with sscanf()
. If the parse with sscanf()
fails to read the data you want -- ignore the line and get the next, e.g.
#define TSIZE 64
...
score *readfile (FILE *fp, score **ptr_s_list, size_t *nelem)
{
...
char buf[TSIZE * TSIZE]; /* buffer to hold each line */
...
/* read a line of input at a time into buf */
while (fgets (buf, sizeof buf, fp)) {
score s; /* temporary stuct to fill */
/* parse record using sscanf from buf, ALWAYS use field-width
* modifier when read into an array to protect array bounds.
*/
if (sscanf (buf, "%d %63s %d %d %d %d %lf", &s.no, s.name,
&s.score[0], &s.score[1], &s.score[2],
&s.sum, &s.avg) != 7) {
continue; /* if parse fails, ignore line, get next */
}
...
This greatly simplifies reading the file and will make your code far more robust.
Handling Allocation/Reallocation
To begin, in C, there is no need to cast the return of malloc
, it is unnecessary. See: Do I cast the result of malloc?. While you are reading and parsing lines of data from your file, you could reallocate once per-line, but that is a bit expensive (less so with a mmap backed malloc, but still relatively expensive). So instead, simply keep two counters, (1) holding the number of structs used
and (2) a second holding the number of structs allocated and available avail
. You only need to reallocate when used == avail
.
When you reallocate, you always realloc()
using a temporary pointer. Why? When (not if) you run out of memory and realloc()
fails, it will return NULL
. If you are reallocating using your pointer, e.g. *ptr_s_list = realloc (*ptr_s_list, ...)
you have just overwritten your pointer address with NULL
preventing you from being able to free that memory creating a memory-leak.
When realloc()
fails it doesn't free()
the original block -- meaning your data is still good. If you overwrite your pointer with NULL
you have also lost your data. Lesson: "Always reallocate to a temporary pointer".
With that (and reading between the lines looking at your *ptr_s_list
and presuming that is due to you passing the address-of the ptr_s_list
pointer to your read function), then when reading, on a successful parse, you can simply check if a reallocation is needed before assigning your temporary struct s
you filled to your allocated list, e.g.
score *readfile (FILE *fp, score **ptr_s_list, size_t *nelem)
{
size_t used = *nelem, /* element counter */
avail = *nelem; /* allocated elements available */
...
void *tmp = NULL; /* temp pointer to realloc with */
/* read a line of input at a time into buf */
while (fgets (buf, sizeof buf, fp)) {
...
/* test if realloc needed */
if (used == avail) {
if (!avail) { /* if first allocation, set to 1 */
avail = 1;
}
/* always realloc to a temporary pointer, below doubles size */
tmp = realloc (*ptr_s_list, 2 * avail * sizeof **ptr_s_list);
/* validate EVERY allocation */
if (!tmp) { /* handle error */
perror ("realloc-*ptr_s_list-readfile()");
if (!used) { /* if no records, return NULL */
return NULL;
}
*nelem = used; /* update number of elements */
return *ptr_s_list; /* return pointer to 1st stuct */
}
avail *= 2; /* update available stuct count */
*ptr_s_list = tmp; /* assign new block to ptr */
}
(*ptr_s_list)[used++] = s; /* assign temp struct to element */
}
Above, note how realloc()
is used for the first and all subsequent reallocations. If your original pointer is NULL
, then realloc()
behaves like malloc()
allowing you to simplify your allocation scheme. Also note, if allocating the first struct, avail
is set to 1
and from then on, the number of available structs is simply doubled when more are needed. You can grow your allocation by whatever method you like, but doubling is a fair balance between reallocation frequency and allocation growth -- up to you.
Above, by passing a pointer to the number of elements and reallocating from there on, you can also call readfile()
multiple time perhaps passing different files to be read while accumulating all of the data in your allocated block of structs. This isn't mandatory, but provides flexibility in how you can use the readfile()
function. By returning the pointer to the allocated block of struct, it also allows you to assign different reads to different pointers if you needed to keep track of separate groups of scores. Again, not mandatory, but simply providing flexibility in how it can be used.
Lastly, and optional, before the readfile()
function returns it calls realloc()
a final time to shrink the allocation to just the number of struct used. (the compiler isn't guaranteed to honor the reduction, but most will) You can do that, again using a temporary pointer, as:
/* optional - reduce size to used structs */
tmp = realloc (*ptr_s_list, used * sizeof **ptr_s_list);
if (!tmp) { /* handle error - warn */
perror ("realloc-*ptr_s_list-sizetofit-readfile()");
}
else {
*ptr_s_list = tmp; /* assign new block to ptr */
}
Adding a short print function to output all stored records and one way to implement reading and storing all data from your file would be:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define TSIZE 64
typedef struct {
int no;
char name[TSIZE];
int score[3];
int sum;
double avg;
} score;
/* funtion to read all scores from file pointer, reallocating
* *ptr_s_list as needed, updating the pointer nelem with the
* number of elements read, retuning a pointer to the allocated
* block of score on success, NULL if no records stored.
*/
score *readfile (FILE *fp, score **ptr_s_list, size_t *nelem)
{
size_t used = *nelem, /* element counter */
avail = *nelem; /* allocated elements available */
char buf[TSIZE * TSIZE]; /* buffer to hold each line */
void *tmp = NULL; /* temp pointer to realloc with */
/* read a line of input at a time into buf */
while (fgets (buf, sizeof buf, fp)) {
score s; /* temporary stuct to fill */
/* parse record using sscanf from buf, ALWAYS use field-width
* modifier when read into an array to protect array bounds.
*/
if (sscanf (buf, "%d %63s %d %d %d %d %lf", &s.no, s.name,
&s.score[0], &s.score[1], &s.score[2],
&s.sum, &s.avg) != 7) {
continue; /* if parse fails, ignore line, get next */
}
/* test if realloc needed */
if (used == avail) {
if (!avail) { /* if first allocation, set to 1 */
avail = 1;
}
/* always realloc to a temporary pointer, below doubles size */
tmp = realloc (*ptr_s_list, 2 * avail * sizeof **ptr_s_list);
/* validate EVERY allocation */
if (!tmp) { /* handle error */
perror ("realloc-*ptr_s_list-readfile()");
if (!used) { /* if no records, return NULL */
return NULL;
}
*nelem = used; /* update number of elements */
return *ptr_s_list; /* return pointer to 1st stuct */
}
avail *= 2; /* update available stuct count */
*ptr_s_list = tmp; /* assign new block to ptr */
}
(*ptr_s_list)[used++] = s; /* assign temp struct to element */
}
*nelem = used; /* update element count */
/* optional - reduce size to used structs */
tmp = realloc (*ptr_s_list, used * sizeof **ptr_s_list);
if (!tmp) { /* handle error - warn */
perror ("realloc-*ptr_s_list-sizetofit-readfile()");
}
else {
*ptr_s_list = tmp; /* assign new block to ptr */
}
return *ptr_s_list; /* return pointer to 1st stuct */
}
/* simple print nelem score records */
void prn_records (score *s, size_t nelem)
{
printf ("\n%zu records:\n\n", nelem);
for (size_t i = 0; i < nelem; i++) {
printf ("%3d %-12s %3d %3d %3d %4d %.1f\n", s[i].no, s[i].name,
s[i].score[0], s[i].score[1], s[i].score[2],
s[i].sum, s[i].avg);
}
}
int main (int argc, char **argv) {
size_t nelem = 0; /* number of elements in allocated block of score */
score *s = NULL; /* pointer to allocated block of score */
/* use filename provided as 1st argument (stdin by default) */
FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;
if (!fp) { /* validate file open for reading */
perror ("file open failed");
return 1;
}
/* read all score records from file, validating at least 1 record read */
if (!readfile (fp, &s, &nelem)) {
fputs ("no records read.\n", stderr);
return 1;
}
prn_records (s, nelem); /* print all stored score records */
free (s); /* free allocated memory */
if (fp != stdin) { /* close file if not stdin */
fclose (fp);
}
}
(note: the program expects the filename of the file to read as the first argument on the command-line, or it will read from stdin
by default if no argument is provided)
Example Use/Output
With the sample data in the file dat/txtscore.txt
as shown above and the program compiled to readtxtscore
in my ./bin
subdirectory, you would get:
$ ./bin/readtxtscore dat/txtscore.txt
3 records:
1 jay 100 90 80 270 90.0
2 key 90 80 65 235 78.3
3 ray 90 45 65 200 66.7
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/readtxtscore dat/txtscore.txt
==11434== Memcheck, a memory error detector
==11434== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==11434== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==11434== Command: ./bin/readtxtscore dat/txtscore.txt
==11434==
3 records:
1 jay 100 90 80 270 90.0
2 key 90 80 65 235 78.3
3 ray 90 45 65 200 66.7
==11434==
==11434== HEAP SUMMARY:
==11434== in use at exit: 0 bytes in 0 blocks
==11434== total heap usage: 6 allocs, 6 frees, 6,456 bytes allocated
==11434==
==11434== All heap blocks were freed -- no leaks are possible
==11434==
==11434== For lists of detected and suppressed errors, rerun with: -s
==11434== 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.
Look things over and let me know if you have questions.