[PATCH] Fix new Coverity IDs

Jeremy Allison jra at samba.org
Thu Jan 12 16:48:41 UTC 2017


On Thu, Jan 12, 2017 at 11:33:30AM +0100, Andreas Schneider wrote:
> On Thursday, 12 January 2017 08:45:58 CET Stefan Metzmacher wrote:
> > Hi Jeremy,
> > 
> > >>> - which uses null_context - which can change what it returns
> > >>> if someone ever calls talloc_enable_null_tracking() !!!!!
> > >> 
> > >> Never mind. As Volker just pointed out to me on the phone,
> > >> null_context = _talloc_named_const(NULL, 0, "null_context"),
> > >> which will always return zero size anyway. Still,
> > >> that code shouldn't be in talloc_get_size().
> > >> 
> > >> Patch(es) to follow !
> > > 
> > > Here the are. Please review and push if happy !
> > > 
> > > (Finally fixed the to: address for metze, sorry).
> > 
> > The talloc_get_size() change is ok.
> > 
> > For the others please use
> > 
> > if (prids != NULL) {
> > 	*prids = NULL;
> > }
> > 
> > at the beginning and
> > 
> > if (prids != NULL) {
> > 	*prids = rids;
> > }
> > 
> > at the end or
> > 
> > *prids = NULL
> > 
> > at the beginning and
> > 
> > *prids = rids;
> > 
> > at the end.
> > 
> > But doing just
> > 
> > if (prids) {
> > 	*prids = rids;
> > }
> > 
> > is ugly and you're the one who always preach
> > that we should not leave pointers uninitialized,
> > so I think we should always set it to NULL at the
> > beginning.

I always preach pointers should be initialized
*AT THE POINT OF DECLARATION*.

Note that prids here isn't owned by the called
function, but the caller, so that's where it
should be initialized (and is).

My rule is that if a called function can error
out, then it shouldn't be assigning to the
any returned pointers - they should only be
touched on success (this is also the API
rule followed by most of POSIX, btw).

> 
> 
> uint32_t *rids;
> 
> 
> needs to be initialized with NULL!
> 
> uint32_t *rids = NULL;
> 
> or the TALLOC_FREE() in 'done' might crash! Which just happend here!

Yeah, that's just my mistake, sorry. I'll
fix and re-submit :-).



More information about the samba-technical mailing list