Re: Dangerous assert() usage, parsed value count and stdbool.h compatibility

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Re: Dangerous assert() usage, parsed value count and stdbool.h compatibility

Martin Hedenfalk-4
3 nov 2010 kl. 16.56 skrev Jens Rehsack:

> Hi Martin, hi Nathan,

Hello,

> because confuse-devel@ still denies mails from non-members, I continue
> to cc you.
>
> I currently stepped into several "issues" in libConfuse - but before
> spending effort on
> fixing them, I'd like to read the maintainers/authors opinion.
>
> 1) I've seen during some code checking, that only assert() is used to prove for
>   NULL-pointers. But assert is expanded to a no-op when NDEBUG is defined,
>   which might be default for some production compiles.
>   Is it intended that in such circumstances (e.g. typo in opt-name when calling
>   cfg_set_validate_func) libconfuse simply segfaults?

If -DNDEBUG is used (why would anyone want to disable error checking!?), I guess it's deliberate. IMHO, the behaviour of having -DNDEBUG disable assert was a mistake.


>   I suggest a macro 'assure(check, error-value)' which returns
> error-value if the
>   test fails (in case of -DNDEBUG). Probably abort() after displaying an error
>   could be suitable, too (even I this is definitively not a preferred way ...).

My intention with aborting was to make it easier to spot errors in calling the library.


> 2) struct cfg_opt_t.nvalues is documented as the number of parsed values, but
>   it's used as the amount of values in struct cfg_opt_t.values.
>   I miss a call to detect whether an option was included in the
> configuration file
>   or not (e.g. to differ if one wants to suppress logging by set
> log-file = "" or if
>   simply no entry of log-file was included in the parsed config).
>   As workaround I use currently a null pointer as default value for
> strings, but
>   this doesn't work for sections.
>
>   My suggestions would be to have a separate counter for parsed values in
>   struct cfg_opt_t.
>
> 3) libConfuse has an incompatible boolean type compared to stdbool.h.
>   I'd like to see there some defines to the stdbool.h boolean types and values,
>   when available - and the libConfuse own own ones only on compilers
>   before C99.

IIRC, recent commits made libconfuse dependent on C99. So either a simple int or the bool from stdbool.h seems reasonable.

> I can spent some effort to fix above issues if you want.

Nathan or Carlo ?

> Cheers,
> Jens



_______________________________________________
Confuse-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/confuse-devel