[PATCH] Do not reset the secrets.tdb SID and GUID when not required

Michael Adam obnox at samba.org
Tue May 20 08:28:44 MDT 2014


On 2014-05-20 at 09:21 +1200, Andrew Bartlett wrote:
> On Mon, 2014-05-19 at 22:55 +0200, Volker Lendecke wrote:
> > On Wed, May 14, 2014 at 05:33:59PM +1200, Andrew Bartlett wrote:
> 
> > Also, a style comment:
> > 
> > > +	if (ret) {
> > > +		if (dom_sid_equal(get_global_sam_sid(), sid) == false) {
> > 
> > I'd rather write !dom_sid_equal(). Any reason why you chose
> > == false?
> 
> I won't claim to have put major thought into it, but it seems clearer,
> particularly compared with the pattern of _cmp() routines that return <
> or > 0.

Well I strongly support Volker here in that "dom_sid_equal"
is of type bool and named so ideomatically that the most
natural reading is "if (!dom_sid_equal(...)) { ..."
(since we don't have perl's "unless (...)" ;-)

If a value is numerical instead of boolean, I always
favour if (foo != NULL) or if (foo == 0)  over
if (!foo) or if (foo), but this bool type is different.

The only additional style-improvement would be to use
a helper variable instead of calling the function
in the if (...) statement:

  ok = dom_sid_equal();
  if (!ok) { ... }

Less ideomatic but more debuggable.

Just my 2¢...

Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140520/d1a913cb/attachment.pgp>


More information about the samba-technical mailing list