On 05/02/2012 05:05 PM, Jakub Hrozek wrote:
On Wed, May 02, 2012 at 08:12:06AM -0400, Stephen Gallagher wrote:
On Thu, 2012-04-26 at 13:51 +0200, Jakub Hrozek wrote:
I scanned the SSSD source code with the clang static analyzer and found
a number of issues. Patches are attached -- I think that most of them
are OK to just include in master. Patches #1, #13 and #16 may be
something that we want to include sooner.

Because the clang analyzer is freely available in Fedora, I think it would
make sense to run a scan at least before a release.

Developers can also run the clang tool themselves - I'll add info how
into our Developers page.

Patch 0004: Nack
There is no NULL-termination guarantee here. We only initialize pid_str
with the first element being NULL.
This is the pid_str initializer:
    char pid_str[MAX_PID_LENGTH] = {'\0'};

I think that this construct initializes the *whole* array with zeroes,
because if there is an initializer but the number of initializers is less
than the number of array elements, the rest of the array elements are set
to 0.

I am not sure it does.  I remember seeing it not being fully initialized.


Patch 0010: Nack
If we're reporting ret (which is an errno_t value) I'd prefer if the
DEBUG messages included the strerror() conversion, since it's much
easier to read.

Patch 0014: Nack
This is not the only case in ipa_get_autofs_options(), but returning
here leaks tmp_ctx and anything still attached to it. Please fix all
such cases here.


Patch 0018: Nack
A tevent_req *_send() function should only return NULL if the
tevent_req_create() fails. Otherwise, all other failures should be
reported with tevent_req_error() and tevent_req_post().

Fixed, but frankly, there's tens of _send() functions in SSSD that just
return NULL on any failure.
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel

Thank you,
Dmitri Pal

Sr. Engineering Manager IPA project,
Red Hat Inc.

Looking to carve out IT costs?