prs_init() and unchecked return values

Jeremy Allison jra at samba.org
Tue Jan 22 22:46:18 GMT 2008


On Tue, Jan 22, 2008 at 02:15:52PM -0800, Marc VanHeyningen wrote:
> 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?

I'm ok with the prs_init_empty() approach. We need to get the
alloc failures caught anyway and this is one way of explicitly
calling them out.

Thanks for the help on this !

Jeremy.


More information about the samba-technical mailing list