prs_init() and unchecked return values

Marc VanHeyningen marc.vanheyningen at isilon.com
Tue Jan 22 22:15:52 GMT 2008


Hello,

Allow me to introduce myself; I'm Marc VanHeyningen, a new-ish guy on
the team responsible for Samba (among other things) at Isilon.  I've
dabbled with Samba in the past but much of it is still new to me.

That said, we've paid for Coverity in name of improving quality (and
planning to push the fixes back) and I'm trying to knock off some of the
issues it raises.  One common "defect" it cites is that prs_init()
returns a BOOL (which indicates whether a malloc failed) but that return
value is frequently ignored.

I could simply add error cases for all of these paths, but roughly half
the calls to prs_init() have the size argument hard-coded to zero; in
this case the function can't fail because no allocation is done.  That
would mean a lot of needless error cases with cleanup functions that are
dead code (but that might be used as a model for future code changes.)
Alternately, I could simply keep those "ignore" values in the code and
either mark them "safe" or try to create a more sophisticated Coverity
model that would ignore them, but that doesn't seem like the most
maintainable approach.

Anyway, I was trying to think of a better way.  Another approach would
be to make a different function (or simulate one with the preprocessor)
for prs_init() when it's invoked with a zero size, say something like
prs_init() and prs_init_alloc() or perhaps prs_init() and
prs_init_empty().  The simplest implementation would be to just define
it something like this:

  #define prs_init_empty( ps, ctx, io ) (void) prs_init(ps, 0, ctx, io)

And then make the appropriate changes where prs_init() is invoked.

Good idea, bad idea, anyone feel strongly?
Tx!
Marc



More information about the samba-technical mailing list