Re: libsmartcols patch

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

On 16 Feb 2015 11:27, Karel Zak wrote:
> On Mon, Feb 16, 2015 at 12:00:53AM -0500, Mike Frysinger wrote:
> > On 28 Dec 2014 20:30, Phillip Susi wrote:
> > > --- libsmartcols/src/table_print.c	2014-12-24 23:59:55.780110296 +0200
> > > +++ libsmartcols/src/table_print.c	2014-12-28 22:45:20.347285226 +0200
> > >  
> > > +	assert(ln);
> > > +	assert(buf);
> > 
> > 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.

>  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;

looks like over-eagerness has already broken the public API ;).

>  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.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]