s4:lsa RPC server - always initialise "info" structures

[Andrew Bartlett]

On Sat, 2010-12-04 at 00:18 +0100, Jelmer Vernooij wrote:
> Hi Matthias,
> 
> On Fri, 2010-12-03 at 23:57 +0100, Matthias Dieter Wallnöfer wrote:
> > commit d0b39324471e5226613a86aad313557cd4a89a9a
> > Author: Matthias Dieter Wallnöfer <mdw at samba.org>
> > Date:   Fri Dec 3 22:47:21 2010 +0100
> > 
> >     s4:lsa RPC server - always initialise "info" structures
> >     
> >     This should help to fix bug #7769
> I'm not sure if this (zeroing out the structs) is the right thing to do.
> The struct members that were previously uninitialised are now perhaps
> initialised - but set to zero. We should really be making sure that all
> members are set explicitly. Which ones were we missing here?
> 
> One of the reasons we don't zero out variables sometimes is so valgrind
> will detect that we haven't fully initialised them when e.g. sending
> them over the wire.

Jelmer,

I have to say that I disagree, but thank you for bringing this up on the
list.  

I do remember when this was the was the rationale, but I've since
changed my view, mostly in response to talking to tridge (CC'ed) about
it.  

I've been convinced that for most things, 0 is an appropriate default,
and we run and analyse the output of valgrind so rarely over the whole
codebase that the benefits in simply not having these errors crop up in
unusual production settings outweigh the costs of not being able to
track these down with valgrind. 

As a good, separate, example see the cli_credentials_init():  It tried
to init the structure manually, but it's got quite silly with how many
initialisers there are, almost all to 0, and should just be a left with
a talloc_zero().

That said, having found this particular case, it would be good to
validate that 0 is the right default here, before doing a blind
talloc_zero().

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20101204/0d590b14/attachment.pgp>