patterncMinor
Parsing an ini file
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
inifile.c
```
#include "inifile.h"
#include
#include
#include
#include
#include
struct inifile_entry
{
const char* entry_nam
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
-
Casting from
-
You can save type-name repetition in places like this:
By instead using the variable name in the
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
-
I personally find impractical prefixing every instantiation of a structure with
-
You have a few formatting issues in your code, which hurt readability. A tool like Clang Format can automate fixing that.
-
Adding curly braces
-
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.