HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Parsing an ini file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fileparsingini

Problem

I have written a basic ini file parser in C. It won't support big ini files, and isn't very fast when fetching configuration values, but should be enough for small ones.

I couldn't find "proper" definition of ini file standard, so there is no handling screening symbols, and all and any error encountered during parsing is silently swallowed.

inifile.h

/** \file inifile.h
 * Functions for reading configuration from disk
 * Configuration options have single lines
 * Comments start with ';'
 */

#ifndef _INIFILE_H_
#define _INIFILE_H_

typedef void* pinifile_data;

/** Open and parse an ini file
 * \param[in] fname name of file to read
 * \param[out] pinidb pointer to the variable for storing reference for
 * inifile data
 * \returns 0 on success
 */
int open_ini_file(const char* fname, pinifile_data* pinidb);

int get_integer(const pinifile_data inidb, const char* section, const char* entry, int default_value);
double get_real(const pinifile_data inidb, const char* section, const char* entry, double default_value);

/** Free memory used by ini file database
 * \param[in] inidb ini file database from open_ini_file
 */
void close_ini_file(pinifile_data inidb);

/** Create ini file database */
pinifile_data create_inidb();

/** Add or change value in ini file database
 * If there is no specified section or entry in database, they will be created
 * \param inidb ini file database
 * \param[in] section name of a section
 * \param[in] entry name of an entry
 * \param[in] value new value of entry in string form
 */
void set_entry(pinifile_data inidb, const char* section, const char* entry, const char* value);

/** Dump ini file database on disc
 * \param fname name of a file to dump into
 * \param inidb ini file database
 * \returns 0 on success
 */
int dump_ini_file(const char* fname, pinifile_data inidb);

#endif // _INIFILE_H_


inifile.c

```
#include "inifile.h"

#include
#include
#include
#include
#include

struct inifile_entry
{
const char* entry_nam

Solution

From a quick look, there are a few things I see that can be improved:

-
Your code has a lot of unnecessary type casts. In special, casting to void is not just useless, but also dangerous, since it might hide an error like passing a value to a function that expects a pointer. Never explicitly cast to void when you call free, memcpy, etc.

-
Casting from void to T is also unnecessary in C. void* converts implicitly both ways. Unless you really care about having the same code compile as C and C++, no need to cast in places like the return value from malloc.

-
You can save type-name repetition in places like this:

pinifile_data create_inidb()
{
    void* result = malloc(sizeof(struct inifile_data));
    memset(result, 0, sizeof(struct inifile_data));
    // Here we are copying a pointer, yes
    return result;
}


By instead using the variable name in the sizeof expression. sizeof is compile-time, so it is legal. E.g.:

pinifile_data create_inidb(void)
{
    struct inifile_data* result = malloc(sizeof(*result));
    memset(result, 0, sizeof(*result));
    return result;
}


Take advantage of the little type inference you have and Don't Repeat Yourself. If you change the name of that type, now it's just one place to update in that function.

But going further, since that function wants to allocate zero-initialized memory, why not use calloc() instead?

pinifile_data create_inidb(void)
{
    return calloc(1, sizeof(struct inifile_data));
}


-
I personally find impractical prefixing every instantiation of a structure with struct, so I always typedef them on declaration to avoid that. Same for enums.

-
You have a few formatting issues in your code, which hurt readability. A tool like Clang Format can automate fixing that.

-
Adding curly braces { } on single line statements is optional, but a good idea. It will make future updates to your code less painful and less error prone.

Code Snippets

pinifile_data create_inidb()
{
    void* result = malloc(sizeof(struct inifile_data));
    memset(result, 0, sizeof(struct inifile_data));
    // Here we are copying a pointer, yes
    return result;
}
pinifile_data create_inidb(void)
{
    struct inifile_data* result = malloc(sizeof(*result));
    memset(result, 0, sizeof(*result));
    return result;
}
pinifile_data create_inidb(void)
{
    return calloc(1, sizeof(struct inifile_data));
}

Context

StackExchange Code Review Q#108264, answer score: 4

Revisions (0)

No revisions yet.