INI File configuration handler

Question

I am currently working on a game, and I get the configuration in an .ini file, and I get information with this library https://github.com/benhoyt/inih I would like to improve the readability / performance of this code, as I do not know much about good practices.

include "project.h"

// typedef struct s_server_conf {
//      const char              *ip;
//      unsigned int    port;
// }                            t_server_conf;

// typedef struct       s_db_conf {
//      const char              *host;
//      const char              *user;
//      const char              *passwd;
//      const char              *db;
//      unsigned int    port;
// }                            t_db_conf;

// typedef struct s_conf {
//      t_server_conf   server;
//      t_db_conf               conf;
// }                            t_conf;

static int      parse_server(t_conf *conf, const char *name, const char *value)
{
        # define MATCH(n) (strcmp(name, n) == 0)

        if (MATCH("ip"))
                conf->server->ip = strdup(value);
        else if (MATCH("port"))
                conf->server->port = atoi(value);
        else
                return (0);
        return (1);
}

static int      parse_database(t_conf *conf, const char *name, const char *value)
{
        # define MATCH_NAME(n) (strcmp(name, n) == 0)

        if (MATCH_NAME("host"))
                conf->db->host = (strdup(value));
        else if (MATCH_NAME("user"))
                conf->db->user = (strdup(value));
        else if (MATCH_NAME("passwd"))
                conf->db->passwd = (strdup(value));
        else
                return (0);
        return (1);
}

static int      handler(void *conf, const char *section, const char *name, const char *value)
{
        # define MATCH_SECTION(s) (strcmp(section, s) == 0)

        printf("Le nom du champ est %s, la valeur est %s (La section est %s)\n", name, value, section)
;

        if (MATCH_SECTION("database"))
                return (parse_database((t_conf*)conf, name, value));
        if (MATCH_SECTION("server"))
                return (parse_server((t_conf*)conf, name, value));
        return (0);
}

void            clear_conf(t_conf *conf)
{
        if (conf->db)
                free(conf->db);
        if (conf->server)
                free(conf->server);
        free(conf);
}

static  t_conf          *create_conf(void)
{
        t_conf  *conf;

        if (!(conf = (t_conf*)malloc(sizeof(t_conf))))
                return (NULL);
        memset(conf, '\0', sizeof(t_conf));
        if (!(conf->db = (t_db_conf*)malloc(sizeof(t_db_conf))))
        {
                clear_conf(conf);
                return (NULL);
        }
        conf->db->port = 3306;
        if (!(conf->server = (t_server_conf*)malloc(sizeof(t_server_conf))))
        {
                clear_conf(conf);
                return (NULL);
        }
        return (conf);
}
t_conf          *get_conf(void)
{
        t_conf  *conf;

        if (!(conf = create_conf()))
                return (NULL);
        if (ini_parse("settings.ini", handler, conf) < 0)
        {
                printf("Unable to load configuration\n");
                exit (1);
        }
        return (conf);
}

Answer 1

Design

Rather than bury a magic filename, consider passing it in.

//t_conf          *get_conf(void) {
//  ... 
//    if (ini_parse("settings.ini", handler, conf) < 0)

t_conf *get_conf(const char *ini_file_name) {
  ... 
  if (ini_parse(ini_file_name, handler, conf) < 0)

Maybe add before the if()

  if (ini_file_name == NULL) {
    ini_file_name = "settings.ini";
  }

Devoid of overall comments

Consider comments to at least convey the coding goals.

Allocate to the size of the de-referenced object

... rather than allocate to the size of the type - easier to code right, review and maintain. Cast not needed.

//if (!(conf = (t_conf*)malloc(sizeof(t_conf))))
//            return (NULL);
conf = malloc(sizeof *conf);
if (conf == NULL) {
  return NULL;
}

// like-wise
// memset(conf, '\0', sizeof(t_conf));
memset(conf, 0, sizeof *conf);

Use auto formatting

Life is too short for manual formatting. Find and use an auto formatter - customized as needed.

Comments of typedefs not needed

As is now, this code has comments that is not required to match what is truly in "project.h" and as time goes on, can diverge. For code posting here, post a portion of "project.h" instead.

Invalid code

include "project.h" should be #include "project.h"

Hiding objects here is bad

# define MATCH(n) (strcmp(name, n) == 0) brings in name, but not obvious from the call if (MATCH("ip")).

Alternative:

static bool streq(const char *a, const char *b) {
  return strcmp(a,b) == 0;
}
...
if (streq(name, "ip"))

Naked magic number

Buried in code is

conf->db->port = 3306;

Better to have

// near top of code
#define PORT_DEFAULT 3306

// later on
conf->db->port = PORT_DEFAULT;

Unneeded if

Just call free(). free(NULL) is OK.

//if (conf->db)
//            free(conf->db);
free(conf->db);

Like free(), I'd expect clear_conf(NULL) to not error

void clear_conf(t_conf *conf) {
  if (conf) { // add  

() not needed in return and others

Many places

// return (conf);
return conf;

// conf->db->host = (strdup(value));
conf->db->host = strdup(value);

Non-standard C

Note: strdup() is not standard, yet ubiquitous.

Missing #include <>

Do not rely on "project.h" to provide them unless that is clearly a project rule.

NULL as name

Given the potential generic use of handler, I'd guard against NULL.

// printf("Le nom du champ est %s, la valeur est %s (La section est %s)\n", name, value, section);
if (name === NULL || value == NULL || section == NULL) {
  tbd();
} else {
  printf("Le nom du champ est %s, la valeur est %s (La section est %s)\n", name, value, section);
}

Further I'd bracket such strings to help detect leading/trailing white spaces in them

  printf("Le nom du champ est \"%s\", la valeur est \"%s\" (La section est \"%s\")\n",
      name, value, section);

Lack of error checking

// what if `value` is "", "abc", "123xyz", "12345678901234567890", ...?
conf->server->port = atoi(value);

// what if allocation returns NULL? 
conf->server->ip = strdup(value);

Respect presentation width

        printf("Le nom du champ est %s, la valeur est %s (La section est %s)\n", name, value, section)
;

vs.

        printf("Le nom du champ est %s, la valeur est %s (La section est %s)\n",
            name, value, section);

Another reason for an auto formatter.

Curious dual language

I'd expect one.

printf("Le nom du champ est %s, la valeur est %s (La section est %s)\n", ...
printf("Unable to load configuration\n");

Conversion changes sign-ness

unsigned port;
...
conf->server->port = atoi(value);

Perhaps a helper function

// return error status
bool to_unsigned(unsigned *u, const char *s) {
  if (s == NULL) {
    return true;
  }
  char *endptr;
  errno = 0;
  unsigned long = strtoul(s, &endptr, 10); 
  if (errno || s == endptr || *endptr || value > UINT_MAX) {
    return true;
  }

  if (u) {
    *u = (unsigned) value;
  }
  return false;
}

if (to_unsigned(&conf->server->port, value)) {
  hanlde_oops();
}