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:
signature.asc
Description: Digital signature
