[PATCH] ctdb: Coverity fix for CID 1291643

Michael Adam obnox at samba.org
Tue Mar 31 10:28:11 MDT 2015


On 2015-03-31 at 08:57 -0700, Jeremy Allison wrote:
> On Tue, Mar 31, 2015 at 11:43:53AM -0400, Ira Cooper wrote:
> > On Tue, Mar 31, 2015 at 11:41 AM, Michael Adam <obnox at samba.org> wrote:
> > 
> > > On 2015-03-31 at 17:32 +0200, David Disseldorp wrote:
> > > > Hi Rajesh,
> > > >
> > > > On Tue, 31 Mar 2015 09:59:15 -0400 (EDT), Rajesh Joseph wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > This is a fix for coverity bug CID 1291643.
> > > > > Please review the patch and let me know if you have any comments.
> > > >
> > > > Looks good, but I think a check for ctdb->name talloc_asprintf()
> > > > failures should also be added alongside this change...
> > > >
> > > > 355         ctdb->name = talloc_asprintf(ctdb, "%s:%u",
> > > > 356                                      ctdb_addr_to_str(ctdb->address),
> > > > 357
> > > ctdb_addr_to_port(ctdb->address));
> > > > 358         DEBUG(DEBUG_INFO,("ctdb chose network address %s\n",
> > > ctdb->name));
> > >
> > > I'd say this would be a second patch, since it is
> > > not the topic of that CID.
> > >
> > > > Also, please limit lines to 80 chars
> > >
> > > Ouch, that escaped me... :)
> > >
> > > > and avoid using Yoda notation -
> > > > most sane compilers catch bogus assignments in if conditions nowadays.
> > >
> > > That's a matter of taste, imho. And is 'most' enough?
> > > I remember that we discussed this in the past, but
> > > I don't remember the outcome...
> > 
> > 
> > I've had patches bounced for Yoda...
> > 
> > And I'd agree on the second patch, because it is another CID.
> 
> What I do in these circumstances is just rewrite the
> patch to be within our guidelines and submit (with
> the appropriate reviewed-by).
> 
> The core of the change is good, and I don't want
> to bounce people the first time for minor issues.

I prefer to avoid silent rewrites unless rewrite is
requested or the submitter is clearly annoyed.

If the person is to come back with more patches
(as is the case here, I am convinced..), then this
initial ping-pong on the patch cosmetics is
exactly the kind of education/introduction to the
samba-coding/patch style, that this person needs.

Next time, he will already know most of the things,
which is not true if the things are silently adapted.

But different people, different approaches... :)

Cheers - Michael

> Subsequent times we can get more picky, as we
> then have a 'relationship' :-).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150331/6ae95ba3/attachment.pgp>


More information about the samba-technical mailing list