samba-technical@lists.samba.org
[Top] [All Lists]

Re: svn commit: samba r19486 - in branches/SAMBA_3_0/source/lib/ldb/comm

Subject: Re: svn commit: samba r19486 - in branches/SAMBA_3_0/source/lib/ldb/common: .
From: Andrew Bartlett
Date: Wed, 25 Oct 2006 08:47:29 +1000
On Tue, 2006-10-24 at 18:37 -0400, simo wrote:
> On Wed, 2006-10-25 at 07:31 +1000, Andrew Bartlett wrote:
> > On Tue, 2006-10-24 at 20:20 +0000, vlendec@xxxxxxxxx wrote:
> > > Author: vlendec
> > > Date: 2006-10-24 20:20:39 +0000 (Tue, 24 Oct 2006)
> > > New Revision: 19486
> > > 
> > > WebSVN: 
> > > http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=19486
> > > 
> > > Log:
> > > Probably Coverity is wrong here, but this fixes their ID 317. Not sure 
> > > whether
> > > to merge it to 4_0. I want it in 3_0 because it took a bit to persuade 
> > > myself
> > > that el can not be NULL here.
> > > 
> > > Volker
> > 
> > > Changeset:
> > > Modified: branches/SAMBA_3_0/source/lib/ldb/common/ldb_msg.c
> > > ===================================================================
> > > --- branches/SAMBA_3_0/source/lib/ldb/common/ldb_msg.c    2006-10-24 
> > > 20:15:13 UTC (rev 19485)
> > > +++ branches/SAMBA_3_0/source/lib/ldb/common/ldb_msg.c    2006-10-24 
> > > 20:20:39 UTC (rev 19486)
> > > @@ -209,7 +209,9 @@
> > >   ret = ldb_msg_add_value(msg, attr_name, val);
> > >   if (ret == LDB_SUCCESS) {
> > >           struct ldb_message_element *el;
> > > -         el = ldb_msg_find_element(msg, attr_name);
> > > +         if (!(el = ldb_msg_find_element(msg, attr_name))) {
> > > +                 return LDB_ERR_OPERATIONS_ERROR;
> > > +         }
> > >           talloc_steal(el->values, val->data);
> > >   }
> > >   return ret;
> > 
> > I can't see how it could ever be NULL.  If ldb_msg_add_value returns
> > LDB_SUCCESS, then an attribute by that name exists, and
> > ldb_msg_find_element must return it.
> 
> If this kind of operation is used enough, we can think of creating
> another support function that directly returns el on attribute creation
> and use that function instead of the 2 ones here.

I think that would be worthwhile.  It would avoid looping over the
attribute list twice.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Red Hat Inc.                  http://redhat.com
<Prev in Thread] Current Thread [Next in Thread>