Ini file parser project

I would appreciate any opinions and critiques anyone has on my code.

My code allow you to read a ini file.
parser.c

```
#include
#include
#include
#include

#include "parser.h"

// read a the whole line, stop if encouter eof
static char readline(FILE in, sip_error *error)
{
size_t size = DEFAULT_LINE_SIZE;
char *str;

if((str = malloc(sizeof str size)) == NULL) {
*error = ERROR_MALLOC;
return NULL;
}

char *pos;
char *temp_alloc;
char lread = 0;

while(fgets(str + size - DEFAULT_LINE_SIZE, DEFAULT_LINE_SIZE, in)) {
lread = 1;
if((pos = strchr(str + size - DEFAULT_LINE_SIZE, '\n')) != NULL) {
*pos = '\0';
break;
}

size = size + DEFAULT_LINE_SIZE - 1;
temp_alloc = realloc(str, sizeof str size);

if(temp_alloc == NULL) {
*error = ERROR_MALLOC;
free(str);
return NULL;
}
}

if(!lread) {
free(str);
return NULL;
}

return str;
}

// strip line for extra spaces and comments
static void line_strip(char *str)
{
char *iterstr = str;
char sstr, estr;

while(iterstr && isspace(iterstr))
++iterstr;

sstr = iterstr;

while(*iterstr) {
if(!isspace(*iterstr)) {
estr = iterstr;
if(estr == '#' || estr == ';') {
--estr;
break;
}
}
++iterstr;
}

*(estr+1) = '\0';
memmove(str, sstr, estr - sstr + 2);
estr = str + (estr - sstr);

while(estr > str && isspace(*estr))
--estr;

*(estr+1) = '\0';
}

void sip_parse_file(FILE file, void (handler)(enum line_type type,
char key_section, char value), sip_error *error)
{

char *section;
char *temp_sec;

char *line;

char *pos;
char key, value;

size_t ssize = DEFAULT_SECTION_SIZE;
size_t size;

if((section = malloc(sizeof section ssize))

Design suggestions:

I'll have to agree with what was mentioned elsewhere about the memory allocation strategy. I don't like the possibility of several mallocs/reallocs being called in a single run. The way I see it, you have two main options: either optmize for speed or for memory usage. Currently you have neither.

Frequent reallocation will be slower and will fragment the heap. In a memory constrained environment, fragmenting the little memory you have is the last thing you want, since it just creates dead unusable space.

I would suggest that you either load the whole file up-front, also simplifying logic a lot in the process by getting rid of the allocation/free logic or, use a fixed size stack allocated memory buffer, something like:

char line_buffer[512];
while (fgets(line_buffer, sizeof(line_buffer), file)) 
{
    ...
}

Then you will only need some simple logic to call fgets again if a line can't fit in the buffer. Lines of text are usually short, so with a big enough buffer, chances are that you will not have to repeat a file IO very often for a single line.

Loading the file entirely into memory is of course even simpler and probably the fastest alternative, since you would be performing just one read operation.

Code review:

A few things you could change/improve in this code:

-
Right now, your functions aren't doing any input parameter validation. If the caller passes a null pointer or a closed file, your code will crash silently. Adopt an input validation strategy. You can validate the inputs and return an error or at least assert to help the debugging process.

-
I find short names, such as these:

char *sstr, *estr;

a little confusing and hard to read. start_ptr/end_ptr or str_start/str_end would read easier. Same goes for ssize, which I think stands for section_size.

-
I would recommend always adding a pair of { } on flow-control statements, even for the single line ones. That simplifies maintenance and shields your code from ambiguity caused by bad indentation.

-
The goto inside the while loop in sip_parse_file() is hard to follow. It also looks dangerous because the jump target is inside the loop, with more code after it. I think It would be a good idea putting some effort into replacing it with something more structured. Separating some tasks into local helper functions might help.

-
The line_type enum could be turned into a typedef enum to avoid having to type enum line_type everywhere. E.g.:

typedef enum {
    e_section,
    e_key_value,
} line_type;

-
ERROR_MALLOC, ERROR_PARSE names are easy to collide with other libraries. A name prefix wouldn't be a bad idea. Same goes for the other constants.

-
Your header file seems to be missing an Include Guard.

StackExchange Code Review Q#78671, answer score: 3