Reading a configuration file

Question

I have written the following C code to parse a configuration file for settings to my program. I denote comments with a # and blank lines are allowed. e.g.:

# Total bytes per line
TBPL 70

# bar minimum
NUMIN 10.
# bar maximum
NUMAX 30000.

# input filename
LLIST_NAME foo.txt

Is there anything badly wrong here? I'm used to Python so finding this a bit of a chore...

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_CONFIG_VARIABLE_LEN 20
#define CONFIG_LINE_BUFFER_SIZE 100
#define MAX_LLIST_NAME_LEN 256
#define MAX_OUT_NAME_LEN 256

struct config_struct { 
    int bytes_per_line;
    char llist_name[MAX_LLIST_NAME_LEN];
    double numin, numax;
} config;

int read_int_from_config_line(char* config_line) {    
    char prm_name[MAX_CONFIG_VARIABLE_LEN];
    int val;
    sscanf(config_line, "%s %d\n", prm_name, &val);
    return val;
}
void read_double_from_config_line(char* config_line, double* val) {    
    char prm_name[MAX_CONFIG_VARIABLE_LEN];
    sscanf(config_line, "%s %lf\n", prm_name, val);
}
void read_str_from_config_line(char* config_line, char* val) {    
    char prm_name[MAX_CONFIG_VARIABLE_LEN];
    sscanf(config_line, "%s %s\n", prm_name, val);
}


void read_config_file(char* config_filename, struct config_struct config) {
    FILE *fp;
    char buf[CONFIG_LINE_BUFFER_SIZE];

    if ((fp=fopen(config_filename, "r")) == NULL) {
        fprintf(stderr, "Failed to open config file %s", config_filename);
        exit(EXIT_FAILURE);
    }
    while(! feof(fp)) {
        fgets(buf, CONFIG_LINE_BUFFER_SIZE, fp);
        if (buf[0] == '#' || strlen(buf) < 4) {
            continue;
        }
        if (strstr(buf, "TBPL ")) {
            config.bytes_per_line = read_int_from_config_line(buf);
        }
        if (strstr(buf, "NUMIN ")) {
            read_double_from_config_line(buf, &config.numin);
        }
        if (strstr(buf, "NUMAX ")) {
            read_double_from_config_line(buf, &config.numax);
        }
        if (strstr(buf, "LLIST_NAME ")) {
            read_str_from_config_line(buf, config.llist_name);
        }
    }
    printf("TBPL = %d\n", config.bytes_per_line);
    printf("NUMIN = %f\nNUMAX = %f\n", config.numin, config.numax);
    printf("LLIST_NAME = %s\n", config.llist_name);
}



int main(int argc, char *argv[]) {
    read_config_file(argv[1], config);
    return EXIT_SUCCESS;
}

Answer 1

Your parser will of course only be able to handle very specific input. For example leading whitespace in front of # or passing garbage values as numbers in your config file.

If you want to improve this, I would first start by not ignoring the return value of sscanf.It will return the number of items it could successfully parse. This could also be used to generate useful error messages.

The file that you are opening should be closed with

fclose(fp).

Your read_*_from_config_line methods have a potential buffer overflow issue when the variable length is longer than MAX_CONFIG_VARIABLE_LEN. In this case you can just increase the buffer size, as you know that the strings are not longer than CONFIG_LINE_BUFFER_SIZE but generally you should try to avoid using sscanf and the likes.

Answer 2

struct config_struct { 
    int bytes_per_line;
    char llist_name[MAX_LLIST_NAME_LEN];
    double numin, numax;
} config;

I would encourage the usage of typedef. And this is global variable. If it is not required, then try to avoid it. In this case, i don't see why you would need to make config a global.

sscanf(config_line, "%s %lf\n", prm_name, val);

Since you are already using functions from string library, why don't you look into splitting the string into tokens and parsing it by delimited and ignoring white space. That way if tomorrow you accidentally add a additional white space in your input file, the code won't break( In terms of reliability ). Something like strtok

Answer 3

The signatures for the read_*_from_config_file functions are inconsistent. One of them returns a value and the other two pass their results by a pointer parameter. Choose one style and stick to it with similar functions. They ought to check the return value of sscanf, in which case they could have an error status a return value and a pointer parameter for the result.

There's no reason for you copying the parameter name to a local variable and then throwing it away without using it. You could just use the standard function strchr to find the first space character, and then read the value from there. Alternatively, if you want to support multiple whitespace in a row, or whitespace at the beginning of a line you can use loops like below.

// Skip leading whitespace
while (isspace(*config_line))
    config_line++;

// Skip first word
while (!isspace(*config_line))
    config_line++;

// Skip separating whitespace
while (isspace(*config_line))
    config_line++;

// Read value here

isspace is a standard library function that checks whether a char is whitespace, i.e. newline, tab, space etc.