URL:
https://github.com/SSSD/sssd/pull/140
Title: #140: gcc7 related fixes
fidencio commented:
"""
On Tue, Jan 31, 2017 at 2:25 PM, lslebodn <notifications(a)github.com> wrote:
On (31/01/17 04:06), fidencio wrote:
>On Tue, Jan 31, 2017 at 10:29 AM, lslebodn <notifications(a)github.com>
wrote:
>>
>> On (30/01/17 23:52), fidencio wrote:
>> >fidencio requested changes on this pull request.
>> >
>> >Neither the short log or the log are consistent between the patches.
>> >Please, let's try to follow the git-commit-template.
>> >
>> >Also, there are some questions in two of the patches, which are
basically curiosity.
>> >
>>
>> Could you be more specific?
>> There is just missing ticket in messages. Because ther is not a ticket.
>
>* sssctl: Fix warning may be used uninitialized:
>Component should be in capital letters according to the
git-commit-template
>
git log says something different
sh$ git log --oneline | grep " SSSCTL:" | wc -l
1
sh$ git log --oneline | grep " sssctl:" | wc -l
26
And I cannot see a reason to change it.
sssctl looks better in lowercase. becaus eutility is sssctl and not SSSCTL
I'm in favour of updating the commit template then, as it hasn't been
followed.
But that's *not* related to this patchset.
>* pam_sss: Suppress warning format-truncation
>Same comment
>
the same here.
>* Suppres implicit-fallthrough from gcc 7
>Some kind of comments are recognized by gcc 7
>but they are ignored with -Wimplicit-fallthrough=5
>and only attributes disable the warning.
>
>Please, don't break the lines with 52 characters. Do it on 72 as
recommended.
>I know the recommendation is "do not exceed 72 (...)" and you don't
>exceed. But we're not going to take this path as someone may write a
>commit message with a word per-line just because it doesn't exceed the
>72 characters :-)
>
If it is the only problem then I can change it before pushing patches :-)
Please, do it.
>>
>>
>> >> @@ -434,8 +434,8 @@ static errno_t sssctl_fetch_object(TALLOC_CTX
*mem_ctx,
>> > struct sss_domain_info **_dom)
>> > {
>> > TALLOC_CTX *tmp_ctx;
>> >- struct sysdb_attrs *entry;
>> >- struct sss_domain_info *dom;
>> >+ struct sysdb_attrs *entry = NULL;
>> >+ struct sss_domain_info *dom = NULL;
>> >
>> >I'm curious why just this code path triggered the issue.
>> >We have similar code in a lot of other places (even in the same file).
>> >
>>
>> Could you be more specific?
>> Because sssctl_find_object is called just once and there is something
>> smelly (hyper optimized) there.
>
>Let me try to re-phrase. Is there any other part of the code where we
>can be bite by the very same issue?
>
>In a lot of places we don't initialize the output vars and may be the
>case we end up hitting this very same "smelly hyper optimization" as
>well.
>
I cannot see any other warning related to uninitialized data.
Does it answer your question?
Kinda. Actually, it does, but the question was poorly formulated.
Shall we preventively change this in any other places? (And no, it doesn't
mean we should change it as part of this patch).
"""
See the full comment at
https://github.com/SSSD/sssd/pull/140#issuecomment-276366120