Reputation: 29
Bascially, Im trying to accept a command line text file so that when I run the program as "program instructions.txt", it stores the values listed in instructions. However I am having trouble testing what i currently have, as it is giving me the error "segmentation fault core dumped".
int main(int argc, char* argv[]) {
setup_screen();
setup();
// File input
char textExtracted[250];
FILE* file_handle;
file_handle = fopen(argv[1], "r");
while(!feof(file_handle)){
fgets(textExtracted, 150, file_handle);
}
fclose(file_handle);
printf("%s", textExtracted[0]);
return 0;
}
Inside the text file is
A 1 2 3 4 5
B 0 0
C 1 1
Im just trying to store each line in an array and then print them.
Upvotes: 1
Views: 2919
Reputation: 36082
Some points:
int main(int argc, char* argv[]) {
I suggest you check number of arguments here before proceeding further
setup_screen();
setup();
// File input
char textExtracted[250];
Declaration can be joined but always always check return values from I/O
FILE* file_handle = = fopen(argv[1], "r");
if (NULL == file_handle)
{
perror(argv[1]);
return EXIT_FAILURE;
}
below is not the correct way to read from a file, instead you should try and read from the file first, then check for error/eof/enuff bytes read
// while(fgets(textExtracted,sizeof(textExtracted), 1, file_handle) > 0) {}
while(!feof(file_handle)){
fgets(textExtracted, 150, file_handle);
}
It looks like you think fgets appends to textExtracted when you call it multiple times, it doesn't! every line in the file will overwrite the previously read line. Note also that the \n character is included in your buffer.
But since your file appears to be pretty small, you could read the whole content into your buffer and work with that.
// int size = fread(textExtracted, sizeof(textExtracted), 1, file_handle);
Better is to check the size of the file first and then allocate a buffer with malloc to hold the whole file or read the file character by character and do whatever commands you need to do on the fly. e.g. a switch statement is excellent as a statemachine
switch( myreadchar )
{
case 'A':
break;
case 'B':
break;
...
}
textExtracted[0] is one character, textExtracted is the whole array so instead of
printf("%s", textExtracted[0]);
write
printf("%s", textExtracted);
or even better
fputs(textExtracted, stdout);
return 0;
Upvotes: 2
Reputation: 84551
The problem you present is the classic problem of "How do I read an unknown number of lines of unknown length from a file?" The way you approach the problem in a memory efficient manner is to declare a pointer-to-pointer to char
and allocate a reasonable number of pointers to begin with and then allocate for each line assigning the starting address for the block holding each line to your pointers as you read each line and allocate for it.
An efficient way to do that is to read each line from a file into a fixed buffer (of size sufficient to hold your longest line without skimping) with fgets
or by using POSIX getline
which will allocate as needed to hold the line. You then remove the trailing '\n'
from your temporary buffer and obtain the length of the line.
Then allocate a block of memory of length + 1
characters (the +1
for the nul-terminating character) and assign the address for your new block of memory to your next available pointer (keeping track of the number of pointers allocated and the number of pointers used)
When the number of pointers used equals the number allocated, you simply realloc
additional pointers (generally by doubling the current number available, or by allocating for some fixed number of additional pointers) and you keep going. Repeat the process as many times as needed until you have read all of the lines in your input file.
There are a number of ways to implement it and arrange the differing tasks, but all basically boil down to the same thing. Start with a temporary buffer of reasonable size to handle your longest line (without skimping, just in case there is some variation in your input data -- a 1K buffer is cheap insurance, adjust as needed). Add your counters to keep track of the number of pointers allocated and then number used (your index). Open and validate the file given on the command line is open for reading (or read from stdin
by default if no argument was given on the command line) For example you could do:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAXC 1024 /* if you need a constant, #define one (or more) */
int main (int argc, char **argv) {
char buf[MAXC] = "", /* temp buffer to hold line read from file */
**lines = NULL; /* pointer-to-pointer to each line */
size_t ndx = 0, alloced = 0; /* current index, no. of ptrs allocated */
/* 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;
}
...
With your file open and validated, you are ready to read each line, controlling your read loop with your read function itself, and following the outline above to handle storage for each line, e.g.
while (fgets (buf, MAXC, fp)) { /* read each line */
size_t len; /* for line length */
if (ndx == alloced) { /* check if realloc needed */
void *tmp = realloc (lines, /* alloc 2X current, or 2 1st time */
(alloced ? 2 * alloced : 2) * sizeof *lines);
if (!tmp) { /* validate EVERY allocation */
perror ("realloc-lines");
break; /* if allocation failed, data in lines still good */
}
lines = tmp; /* assign new block of mem to lines */
alloced = alloced ? 2 * alloced : 2; /* update ptrs alloced */
}
Note: above, the first thing that happens in your read loop is to check if you have pointers available, e.g. if (ndx == alloced)
, if your index (number used) is equal to the number allocated, you reallocate more. The ternary above alloced ? 2 * alloced : 2
simply asks if you have some previously allocated alloced ?
then double the number 2 * alloced
otherwise (:
) just start with 2
pointers and go from there. In that doubling scheme, you allocate 2, 4, 8, 16, ...
pointers with each successive reallocation.
Also note: when you call realloc
you always use a temporary pointer, e.g. tmp = realloc (lines, ...)
and you never realloc
using the pointer itself, e.g. lines = realloc (lines, ...)
. When (not if) realloc
fails, it returns NULL
, and if you assign that to your original pointer -- you have just created a memory leak because the address for lines
has been lost meaning you cannot reach or free()
the memory you previously allocated.
Now you have confirmed you have a pointer available to assign the address of a block of memory to hold the line, remove the '\n'
from buf
and get the length of the line. You can do that conveniently in a single call to strcspn
which returns the initial number of characters in the string not containing the delimiter "\n"
, e.g.
buf[(len = strcspn(buf, "\n"))] = 0; /* trim \n, get length */
(note: above you are simply overwriting the '\n'
with the nul-terminating character 0
, equivalent to '\0'
)
Now that you have the length of the line, you simply allocate length + 1
characters and copy from the temporary buffer buf
to your new block of memory, e.g.
if (!(lines[ndx] = malloc (len + 1))) { /* allocate for lines[ndx] */
perror ("malloc-lines[ndx]"); /* validate combined above */
break;
}
memcpy (lines[ndx++], buf, len + 1); /* copy buf to lines[ndx] */
} /* increment ndx */
At that point you are done reading and storing all lines and can simply close the file if not reading from stdin
. Here, for example, we just output each of the lines, and then free
the storage for each line, finally freeing the memory for the allocated pointers as well, e.g.
if (fp != stdin) fclose (fp); /* close file if not stdin */
for (size_t i = 0; i < ndx; i++) { /* loop over each storage line */
printf ("lines[%2zu] : %s\n", i, lines[i]); /* output line */
free (lines[i]); /* free storage for strings */
}
free (lines); /* free pointers */
}
That's it. Putting it altogether, you could do:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAXC 1024 /* if you need a constant, #define one (or more) */
int main (int argc, char **argv) {
char buf[MAXC] = "", /* temp buffer to hold line read from file */
**lines = NULL; /* pointer-to-pointer to each line */
size_t ndx = 0, alloced = 0; /* current index, no. of ptrs allocated */
/* 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;
}
while (fgets (buf, MAXC, fp)) { /* read each line */
size_t len; /* for line length */
if (ndx == alloced) { /* check if realloc needed */
void *tmp = realloc (lines, /* alloc 2X current, or 2 1st time */
(alloced ? 2 * alloced : 2) * sizeof *lines);
if (!tmp) { /* validate EVERY allocation */
perror ("realloc-lines");
break; /* if allocation failed, data in lines still good */
}
lines = tmp; /* assign new block of mem to lines */
alloced = alloced ? 2 * alloced : 2; /* update ptrs alloced */
}
buf[(len = strcspn(buf, "\n"))] = 0; /* trim \n, get length */
if (!(lines[ndx] = malloc (len + 1))) { /* allocate for lines[ndx] */
perror ("malloc-lines[ndx]"); /* validate combined above */
break;
}
memcpy (lines[ndx++], buf, len + 1); /* copy buf to lines[ndx] */
} /* increment ndx */
if (fp != stdin) fclose (fp); /* close file if not stdin */
for (size_t i = 0; i < ndx; i++) { /* loop over each storage line */
printf ("lines[%2zu] : %s\n", i, lines[i]); /* output line */
free (lines[i]); /* free storage for strings */
}
free (lines); /* free pointers */
}
Example Use/Output
$ ./bin/fgets_lines_dyn dat/cmdlinefile.txt
lines[ 0] : A 1 2 3 4 5
lines[ 1] : B 0 0
lines[ 2] : C 1 1
Redirecting from stdin
instead of opening the file:
$ ./bin/fgets_lines_dyn < dat/cmdlinefile.txt
lines[ 0] : A 1 2 3 4 5
lines[ 1] : B 0 0
lines[ 2] : C 1 1
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 insure 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/fgets_lines_dyn dat/cmdlinefile.txt
==6852== Memcheck, a memory error detector
==6852== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==6852== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==6852== Command: ./bin/fgets_lines_dyn dat/cmdlinefile.txt
==6852==
lines[ 0] : A 1 2 3 4 5
lines[ 1] : B 0 0
lines[ 2] : C 1 1
==6852==
==6852== HEAP SUMMARY:
==6852== in use at exit: 0 bytes in 0 blocks
==6852== total heap usage: 6 allocs, 6 frees, 624 bytes allocated
==6852==
==6852== All heap blocks were freed -- no leaks are possible
==6852==
==6852== For counts of detected and suppressed errors, rerun with: -v
==6852== 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.
While allocating storage for each pointer and each line may look daunting at first, it is a problem you will face over and over again, whether reading and storing lines from a file, integer or floating point values in some 2D representation of the data, etc... it is worth the time it takes to learn. Your alternative is to declare a fixed size 2D array and hope your line length never exceeds the declared width and the number of lines never exceeds your declared number of rows. (you should learn that as well, but the limitation become quickly apparent)
Look things over and let me know if you have further questions.
Upvotes: 1