[PATCH] ctdb: Coverity fix for CID 1291643
jra at samba.org
Tue Mar 31 09:57:23 MDT 2015
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.
Subsequent times we can get more picky, as we
then have a 'relationship' :-).
More information about the samba-technical