This is a meaningless statement:
Code:
*(stringbuild + strlen(stringbuild)) = '\0';
If it works, then there must already be a '\0' character at that exact position, otherwise strlen wouldn't know where to stop. strlen is basically something like:
Code:
size_t strlen(const char *str) {
const char *p = str;
while (*p != '\0') ++p; // loop until you hit a '\0'
return p - str; // return how far you had to search
}
You shouldn't be using strlen at all. Instead, just keep a len variable to keep track of the length.
Also, simply pointing the file (filename) pointer to the given input file name is not a good idea since the input may have come from a function's local variable that may cease to exist before you are done with your data structure. It's best to make a copy.
And there's no reason for fd to be global.
Here's a rewrite that seems to work, although I didn't test it much.
Code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct {
char *file;
char **lines;
int lineCount;
} FileData;
void *xrealloc(void *ptr, size_t size) {
void *newptr = realloc(ptr, size);
if (!newptr) { perror("xrealloc"); exit(EXIT_FAILURE); }
return newptr;
}
FileData fileToMem(const char *file) {
FILE *fp = fopen(file, "r");
if (!fp) { perror(file); exit(EXIT_FAILURE); }
FileData fd;
fd.file = strcpy(xrealloc(NULL, strlen(file) + 1), file);
fd.lines = NULL;
fd.lineCount = 0;
char *line = xrealloc(NULL, 1);
line[0] = '\0';
int len = 1;
for (char ch; (ch = fgetc(fp)) != EOF; ) {
if (ch != '\n') {
line[len - 1] = ch;
line = xrealloc(line, ++len);
line[len - 1] = '\0';
} else {
fd.lines = xrealloc(fd.lines, ++fd.lineCount * sizeof(char*));
fd.lines[fd.lineCount - 1] = xrealloc(NULL, len);
strcpy(fd.lines[fd.lineCount - 1], line);
line = xrealloc(line, 1);
line[0] = '\0';
len = 1;
}
}
free(line);
fclose(fp);
return fd;
}
int main() {
const char *input = "test.txt";
FileData fd = fileToMem(input);
printf("File Name: %s - Line Count: %d - Lines:\n", fd.file, fd.lineCount);
for (int i = 0; i < fd.lineCount; i++) {
printf("%s\n", fd.lines[i]);
free(fd.lines[i]);
}
free(fd.lines);
free(fd.file);
return 0;
}
It's actually insanely inefficient to realloc for every character! I would use fgets to read into a non-dynamic buffer and transfer the string to the lines array. A caveat is that if the string in the buffer doesn't end in a newline character, that means that (unless it's end-of-file) there is more of that line in the file, so you'd need to handle that possibility. I may write up an example and post it too.
As for casting malloc/calloc/realloc, since they return a void*, you don't need to cast the result in C (although you do in C++). One reason we don't cast it is to catch the error of forgetting to include stdlib.h. In that case the default return value is int, which would cause an error if you don't cast it, but the cast would cover it up.
It doesn't help with readability since you already know the type that it's being stored to.
Also it's just another meaningless thing that needs to change if you change the type you're storing to. If you write it like this then you don't even need to change the sizeof expression to change the type:
Code:
// Only the initial int needs to be changed to change the type:
int *values = malloc(numValues * sizeof *values);
EDIT: It just occurred to me that if the last line of the file happens to lack a newline character then that line will not be stored. To fix that you could add the following after the loop, before the free:
Code:
if (len > 1) {
fd.lines = xrealloc(fd.lines, ++fd.lineCount * sizeof(char*));
fd.lines[fd.lineCount - 1] = xrealloc(NULL, len);
strcpy(fd.lines[fd.lineCount - 1], line);
}