[Confuse-devel] some steps towards const-correctness

[Peter Cordes]

 libConfuse turned out to be a handy little library for config files for an
embedded software project I'm working on.  Thanks for making such a nice
non-complex bit of code available.

 I like to declare constants as
const static cfg_opt_t serial_opts[] = {
...
};
for example, but libConfuse doesn't declare any cfg_opt_t pointers to const,
so gcc always warns about discarding const.

I don't see how to get around the
  warning: initialization discards qualifiers from pointer target type
from

const static cfg_opt_t foo_opt[] = {...};
const static cfg_opt_t main_opt[] = {
        CFG_SEC("foo", foo_opt, CFGF_MULTI),
        CFG_END()
};

cfg_opt_t::subopts (pardon the C++ notation) can't be a pointer to const,
because that same data structure is used for modifiable opts as for
the config file definition/defaults passed to cfg_init.

But passing a const cfg_opt_t to cfg_init shouldn't be a problem.
It makes a deep copy of the cfg_opt_t array, so there's no reason not to
store your config-file declarations in read-only memory.  Here's a patch that
converts cfg_opt_t* to const cfg_opt_t* in some of the places where that is
appropriate.  It solves my problems (except for the subopts initializer
problem, which I cast to silence), but I'm haven't checked things
thoroughly.  And my code that uses libconfuse doesn't use any
callbacks, just parse and extract.


 Without C++ const/non-const overloads of the same name, it would be
more trouble than its worth to const-up confuse.c any more.  The data
structure having non-const pointers makes it non-trivial to get right,
because the compiler won't stop you from returning non-const pointers
to sub-trees of const pointer args.

 You'd need to enforce things yourself by having pairs of functions like
DLLIMPORT const cfg_t *cfg_opt_getnsec_const(const cfg_opt_t *opt, unsigned int 
index);
DLLIMPORT cfg_t *cfg_opt_getnsec(cfg_opt_t *opt, unsigned int index);
but not
DLLIMPORT cfg_t *foo(const cfg_opt_t *opt, unsigned int index);

i.e. don't return non-const pointers to any child of a const structure.
As I said, the compiler won't enforce it, and const cfg_opt_t[] is
probably most useful for the argument to cfg_init, and not much else.
Hmm, maybe if you have a program that makes extensive use of
libconfuse data structures and wants to verify const-correctness of
its own functions, the notion that a const cfg_t* can't be used to
modify anything under the cfg_t would be useful.  But that seems
unlikely.


You can support the const/non-const pairs by implementing the
non-const one in terms of the const one, with casts:
DLLIMPORT const cfg_opt_t *cfg_getopt(const cfg_t *cfg, const char *name)
{
 // the current implementation of cfg_getopt
}

DLLIMPORT cfg_opt_t *cfg_getopt(cfg_t *cfg, const char *name)
{
        return (cfg_opt_t *)cfg_const_getopt(cfg, name);
}

 And the error-handler function type would have to change to taking
const* args, because if error-handler callbacks are allowed to modify
the cfg_opt_t[], the get* functions can't take const arguments.



 All that said, I have made a patch to add const qualifiers to the
function arguments that can be made const without adding any extra
functions or making it more, not less, confusing.  (i.e. I avoided adding
const to the args of functions that need to return non-const pointers,
like cfg_getopt, which is used for both get and set).

patch against current CVS:

Index: src/confuse.c
===================================================================
RCS file: /sources/confuse/confuse/src/confuse.c,v
retrieving revision 1.41
diff -u -r1.41 confuse.c
--- src/confuse.c       3 Dec 2008 10:45:47 -0000       1.41
+++ src/confuse.c       28 Oct 2009 19:27:38 -0000
@@ -173,28 +173,28 @@
     return 0;
 }
 
-DLLIMPORT const char *cfg_title(cfg_t *cfg)
+DLLIMPORT const char *cfg_title(const cfg_t *cfg)
 {
     if(cfg)
         return cfg->title;
     return 0;
 }
 
-DLLIMPORT const char *cfg_name(cfg_t *cfg)
+DLLIMPORT const char *cfg_name(const cfg_t *cfg)
 {
     if(cfg)
         return cfg->name;
     return 0;
 }
 
-DLLIMPORT const char *cfg_opt_name(cfg_opt_t *opt)
+DLLIMPORT const char *cfg_opt_name(const cfg_opt_t *opt)
 {
     if(opt)
         return opt->name;
     return 0;
 }
 
-DLLIMPORT unsigned int cfg_opt_size(cfg_opt_t *opt)
+DLLIMPORT unsigned int cfg_opt_size(const cfg_opt_t *opt)
 {
     if(opt)
         return opt->nvalues;
@@ -206,7 +206,7 @@
     return cfg_opt_size(cfg_getopt(cfg, name));
 }
 
-DLLIMPORT signed long cfg_opt_getnint(cfg_opt_t *opt, unsigned int index)
+DLLIMPORT signed long cfg_opt_getnint(const cfg_opt_t *opt, unsigned int index)
 {
     assert(opt && opt->type == CFGT_INT);
     if(opt->values && index < opt->nvalues)
@@ -228,7 +228,7 @@
     return cfg_getnint(cfg, name, 0);
 }
 
-DLLIMPORT double cfg_opt_getnfloat(cfg_opt_t *opt, unsigned int index)
+DLLIMPORT double cfg_opt_getnfloat(const cfg_opt_t *opt, unsigned int index)
 {
     assert(opt && opt->type == CFGT_FLOAT);
     if(opt->values && index < opt->nvalues)
@@ -250,7 +250,7 @@
     return cfg_getnfloat(cfg, name, 0);
 }
 
-DLLIMPORT cfg_bool_t cfg_opt_getnbool(cfg_opt_t *opt, unsigned int index)
+DLLIMPORT cfg_bool_t cfg_opt_getnbool(const cfg_opt_t *opt, unsigned int index)
 {
     assert(opt && opt->type == CFGT_BOOL);
     if(opt->values && index < opt->nvalues)
@@ -272,7 +272,7 @@
     return cfg_getnbool(cfg, name, 0);
 }
 
-DLLIMPORT char *cfg_opt_getnstr(cfg_opt_t *opt, unsigned int index)
+DLLIMPORT char *cfg_opt_getnstr(const cfg_opt_t *opt, unsigned int index)
 {
     assert(opt && opt->type == CFGT_STR);
     if(opt->values && index < opt->nvalues)
@@ -293,7 +293,7 @@
     return cfg_getnstr(cfg, name, 0);
 }
 
-DLLIMPORT void *cfg_opt_getnptr(cfg_opt_t *opt, unsigned int index)
+DLLIMPORT void *cfg_opt_getnptr(const cfg_opt_t *opt, unsigned int index)
 {
     assert(opt && opt->type == CFGT_PTR);
     if(opt->values && index < opt->nvalues)
@@ -373,7 +373,7 @@
     return opt->values[opt->nvalues++];
 }
 
-DLLIMPORT int cfg_numopts(cfg_opt_t *opts)
+DLLIMPORT int cfg_numopts(const cfg_opt_t *opts)
 {
     int n;
 
@@ -382,7 +382,7 @@
     return n;
 }
 
-static cfg_opt_t *cfg_dupopt_array(cfg_opt_t *opts)
+static cfg_opt_t *cfg_dupopt_array(const cfg_opt_t *opts)
 {
     int i;
     cfg_opt_t *dupopts;
@@ -1090,7 +1090,7 @@
     return CFG_SUCCESS;
 }
 
-DLLIMPORT cfg_t *cfg_init(cfg_opt_t *opts, cfg_flag_t flags)
+DLLIMPORT cfg_t *cfg_init(const cfg_opt_t *opts, cfg_flag_t flags)
 {
     cfg_t *cfg;
 
Index: src/confuse.h
===================================================================
RCS file: /sources/confuse/confuse/src/confuse.h,v
retrieving revision 1.28
diff -u -r1.28 confuse.h
--- src/confuse.h       6 Oct 2008 13:40:56 -0000       1.28
+++ src/confuse.h       28 Oct 2009 19:27:38 -0000
@@ -535,7 +535,7 @@
  * @return A configuration context structure. This pointer is passed
  * to almost all other functions as the first parameter.
  */
-DLLIMPORT cfg_t * __export cfg_init(cfg_opt_t *opts, cfg_flag_t flags);
+DLLIMPORT cfg_t * __export cfg_init(const cfg_opt_t *opts, cfg_flag_t flags);
 
 /** Parse a configuration file. Tilde expansion is performed on the
  * filename before it is opened. After a configuration file has been
@@ -602,7 +602,7 @@
  * @param index Index of the value to get. Zero based.
  * @see cfg_getnint
  */
-DLLIMPORT signed long __export cfg_opt_getnint(cfg_opt_t *opt, unsigned int 
index);
+DLLIMPORT signed long __export cfg_opt_getnint(const cfg_opt_t *opt, unsigned 
int index);
 
 /** Indexed version of cfg_getint(), used for lists.
  * @param cfg The configuration file context.
@@ -629,7 +629,7 @@
  * @param index Index of the value to get. Zero based.
  * @see cfg_getnfloat
  */
-DLLIMPORT double __export cfg_opt_getnfloat(cfg_opt_t *opt, unsigned int 
index);
+DLLIMPORT double __export cfg_opt_getnfloat(const cfg_opt_t *opt, unsigned int 
index);
 
 /** Indexed version of cfg_getfloat(), used for lists.
  * @param cfg The configuration file context.
@@ -655,7 +655,7 @@
  * @param index Index of the value to get. Zero based.
  * @see cfg_getnstr
  */
-DLLIMPORT char * __export cfg_opt_getnstr(cfg_opt_t *opt, unsigned int index);
+DLLIMPORT char * __export cfg_opt_getnstr(const cfg_opt_t *opt, unsigned int 
index);
 
 /** Indexed version of cfg_getstr(), used for lists.
  * @param cfg The configuration file context.
@@ -681,7 +681,7 @@
  * @param index Index of the value to get. Zero based.
  * @see cfg_getnbool
  */
-DLLIMPORT cfg_bool_t __export cfg_opt_getnbool(cfg_opt_t *opt, unsigned int 
index);
+DLLIMPORT cfg_bool_t __export cfg_opt_getnbool(const cfg_opt_t *opt, unsigned 
int index);
     
 /** Indexed version of cfg_getbool(), used for lists.
  *
@@ -704,7 +704,7 @@
 DLLIMPORT cfg_bool_t __export cfg_getbool(cfg_t *cfg, const char *name);
 
 
-DLLIMPORT void * __export cfg_opt_getnptr(cfg_opt_t *opt, unsigned int index);
+DLLIMPORT void * __export cfg_opt_getnptr(const cfg_opt_t *opt, unsigned int 
index);
 DLLIMPORT void * __export cfg_getnptr(cfg_t *cfg, const char *name, unsigned 
int indx);
 
 /** Returns the value of a user-defined option (void pointer).
@@ -774,7 +774,7 @@
  * 0 will be returned (ie, the option value is not set at all).
  * @param opt The option structure (eg, as returned from cfg_getopt())
  */
-DLLIMPORT unsigned int __export cfg_opt_size(cfg_opt_t *opt);
+DLLIMPORT unsigned int __export cfg_opt_size(const cfg_opt_t *opt);
 
 /** Return the number of values this option has. If no default value
  * is given for the option and no value was found in the config file,
@@ -796,7 +796,7 @@
  * @return Returns the title, or 0 if there is no title. This string
  * should not be modified.
  */
-DLLIMPORT const char * __export cfg_title(cfg_t *cfg);
+DLLIMPORT const char * __export cfg_title(const cfg_t *cfg);
 
 /** Return the name of a section.
  *
@@ -804,7 +804,7 @@
  * @return Returns the title, or 0 if there is no title. This string
  * should not be modified.
  */
-DLLIMPORT const char * __export cfg_name(cfg_t *cfg);
+DLLIMPORT const char * __export cfg_name(const cfg_t *cfg);
 
 /** Return the name of an option.
  *
@@ -812,7 +812,7 @@
  * @return Returns the title, or 0 if there is no title. This string
  * should not be modified.
  */
-DLLIMPORT const char * __export cfg_opt_name(cfg_opt_t *opt);
+DLLIMPORT const char * __export cfg_opt_name(const cfg_opt_t *opt);
 
 /** Predefined include-function. This function can be used in the
  * options passed to cfg_init() to specify a function for including
@@ -1007,7 +1007,7 @@
 DLLIMPORT void __export cfg_setlist(cfg_t *cfg, const char *name,
                                     unsigned int nvalues, ...);
 
-DLLIMPORT int __export cfg_numopts(cfg_opt_t *opts);
+DLLIMPORT int __export cfg_numopts(const cfg_opt_t *opts);
 
 /** Add values for a list option. The new values are appended to any
  * current values in the list.


--
#define X(x,y) x##y
Peter Cordes ;  e-mail: X(address@hidden , des.ca)
Omnitech Electronics

"The gods confound the man who first found out how to distinguish the hours!
 Confound him, too, who in this place set up a sundial, to cut and hack
 my day so wretchedly into small pieces!" -- Plautus, 200 BC

signature.asc
Description: Digital signature