5
\$\begingroup\$

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;
}
\$\endgroup\$
  • \$\begingroup\$ do not assume much of anything about the configuration file, except perhaps that the first word (on a line that is not a comment nor blank) is a keyword. (your program should contain a table of the valid keywords) then 1) check that the config file keyword is valid. 2) execute a small function to handle that keyword \$\endgroup\$ – user3629249 Jul 23 '15 at 6:50
  • 1
    \$\begingroup\$ If you're interested, I coded a very small parser thing in C: github.com/welljsjs/Config-Parser-C This could give you some inspiration. \$\endgroup\$ – Jessie Feb 10 '18 at 20:18
5
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Thanks for this -- I shall certainly check the return value of sscanf. But how would I parse my file's lines for the values I need without using sscanf? \$\endgroup\$ – Tom Jul 22 '15 at 8:33
  • \$\begingroup\$ You could split the string using strtok which operates on the original buffer (therefore safe) and then using strtol and strtod. \$\endgroup\$ – ssuerbier Jul 23 '15 at 5:41
2
\$\begingroup\$
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

\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ You're right -- I realised after posting that I hadn't changed one of the read_*_from_config_file functions. I'll certainly use your whitespace-stripping routine too, thanks. \$\endgroup\$ – Tom Jul 22 '15 at 8:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.