[SSSD] [INI] Patches for ding-libs: Merging config sections, handling metadata, remaining Coverity issues...

Stephen Gallagher sgallagh at redhat.com
Tue Mar 20 12:41:09 UTC 2012


On Mon, 2012-03-19 at 19:01 -0400, Dmitri Pal wrote:
> Thanks for review. I do not like memset().  It just hides things. If
> it is not initialized coverity and valgrind would not complain and we
> will see the issue. 

As I said, I don't care sufficiently about it for me to nack it.
However, I strongly disagree that memset() hides things. It's defensive
coding, as it guarantees that even if someone working in another area of
the code adds a new option to the struct, they won't need to remember to
initialize the variable. It's not safe to rely on static analysis tools
to let us know if a variable isn't initialized. Some of them cannot
follow memory through its entire lifecycle.

In the SSSD code, we have a convention that requires us to allocate
memory for structures using talloc_zero(), which internally is a
malloc() plus a memset() to protect against this.

I'd like to know what you think it "hides", though.

> The plugin for gedit that I use does not show the spaces the way the
> old plugin did. I need to find a good plugin again.
> I will take a note to find a better plugin.

> Please push and I will just rebase. 

I'm going to hold off on pushing until I finish the review of the
remaining nine patches and then test them all together carefully.

I'm aiming to complete this review today.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120320/49c941b4/attachment.sig>


More information about the sssd-devel mailing list