Re: libsmartcols patch

[Karel Zak <kzak@redhat.com>]

On Mon, Feb 16, 2015 at 05:48:48AM -0500, Mike Frysinger wrote:
> > > general note: libraries should never assert/abort/exit.
> > 
> >  what about segfault?
> 
> that's a red herring.  you can't guarantee your codebase is bug free, but that 
> doesn't mean encouraging a policy of committing suicide is a good thing.

suicide + usable message is better than segfault

> >  We use assert to detect fatal internal library bugs, for mistakes in
> >  applications (e.g. incomplete arguments) it returns -EINVAL.
> 
> libsmartcols/src/table_print.c:
> int scols_print_table(struct libscols_table *tb)
> {
> ...
>     assert(tb);
>     if (!tb)
>         return -1;

this is mistake, fixed in libsmartcols and libmount, I'll later fix it
in libfdisk too.

> >  Its' possible to remove CONFIG_LIBSMARTCOLS_ASSERT from smartcolsP.h,
> >  but I don't think the library is already well tested.
> 
> odd ... why is that reimplementing the wheel ?  the standard assert.h already 
> supports NDEBUG for turning off asserts.
> 
> i see there's a bunch of such knobs in the codebase.  time for a 
> --{en,dis}able-asserts configure flag ?  you could delete all the existing 
> CONFIG_xxx_ASSERT defines and simply rely on configure adding -DNDEBUG to 
> CPPFLAGS.

Maybe, the idea has been to keep it in our hands and force people to
use asserts for the "not yet well tested" libs.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com