[sssd PR#378][opened] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: opened

PR body:
"""
This is the 3rd tentative to have this patch reviewed. For more references, please, see:
PR #50.

So, I've re-worked those patches a little bit and here is the time difference when
running reconfing with the patches:
```
real	0m26.047s
user	0m21.318s
sys	0m4.635s
```

And now without:
```
real	0m25.565s
user	0m20.696s
sys	0m4.433s
```

This patch set is rebased on top of PR #377.

I really would appreciate if someone could review and give their opinion.

The reason this PR was blocked is because this time difference has been considered a
"performance issue".

@jhrozek , could you take a look on this?
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][synchronized] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][synchronized] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][synchronized] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][synchronized] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
The only two patches which have to be reviewed as part of this PR are:

- CMOCKA: Silence a false-positive "deference of null pointer" warning
- BUILD: Make use of GNULIB's compiler warning code

The patches which this PR has been rebased atop are here because without then our CI would
fail.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-328278917

[sssd PR#378][synchronized] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

jhrozek commented:
"""
I'm seeing a build failure with these patches on my machine:
```
/home/remote/jhrozek/devel/sssd/src/util/crypto/nss/nss_sha512crypt.c: In function
‘sha512_crypt_r’:
/home/remote/jhrozek/devel/sssd/src/util/crypto/nss/nss_sha512crypt.c:71:12: error: stack
protector not protecting local variables: variable length buffer
[-Werror=stack-protector]
 static int sha512_crypt_r(const char *key,
            ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Makefile:17864: recipe for target
'src/util/crypto/nss/libsss_crypt_la-nss_sha512crypt.lo' failed
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-328800775

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

jhrozek commented:
"""
btw as I said in PR #371 since there are fixes developed independently, maybe the patches
that are also included from PR #371 should be dropped from this PR so that we can merge
both.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-328801163

[sssd PR#378][+Changes requested] [RFC] Use GNULIB's compiler warning code

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

Label: +Changes requested

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

mzidek-rh commented:
"""
I did not review all the patches, but I do not like the removal of the INI warning. I
guess we can not treat warnings as errors for all platforms. It is easy to forget that you
need new ding-libs for some of the features to work properly in SSSD and if someone
compiles the code for EL6 (scratch builds) this warning is a valuable piece of
information. I explicitly grep for it when do some scratch builds to verify if the new
ding-libs was available during compile time (not just run time).
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-328810902

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
Okay. I'll re-work this PR.

About merging this PR and the #371, I'm okay with that. But I'd prefer dropping
the patches from there than from here (as those patches here are at least 6 months older
than that ones).
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-328812923

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

jhrozek commented:
"""
btw there are more warnings on RHEL-6:
```
cache_req_data.o `test -f 'src/responder/common/cache_req/cache_req_data.c' ||
echo
'/var/lib/jenkins/workspace/ci/label/rhel6/'`src/responder/common/cache_req/cache_req_data.c
cc1: warning: command line option "-Wenum-compare" is valid for C++/ObjC++ but
not for C
cc1: warning: command line option "-Wenum-compare" is valid for C++/ObjC++ but
not for C
cc1: warnings being treated as errors
/var/lib/jenkins/workspace/ci/label/rhel6/src/responder/common/cache_req/cache_req_result.c:
In function 'cache_req_add_result':
/var/lib/jenkins/workspace/ci/label/rhel6/src/responder/common/cache_req/cache_req_result.c:35:
error: declaration of 'index' shadows a global declaration [-Wshadow]
/usr/include/string.h:489: error: shadowed declaration is here [-Wshadow]
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-328961312

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
On Tue, Sep 12, 2017 at 9:36 PM, Jakub Hrozek <notifications(a)github.com&gt;
wrote:

>  btw there are more warnings on RHEL-6:
>
>  cache_req_data.o `test -f 'src/responder/common/cache_req/cache_req_data.c' ||
> echo
> '/var/lib/jenkins/workspace/ci/label/rhel6/'`src/responder/common/cache_req/cache_req_data.c
>  cc1: warning: command line option "-Wenum-compare" is valid for C++/ObjC++ but
> not for C
>  cc1: warning: command line option "-Wenum-compare" is valid for C++/ObjC++ but
> not for C
>  cc1: warnings being treated as errors
>
> /var/lib/jenkins/workspace/ci/label/rhel6/src/responder/common/cache_req/cache_req_result.c:
> In function 'cache_req_add_result':
>
> /var/lib/jenkins/workspace/ci/label/rhel6/src/responder/common/cache_req/cache_req_result.c:35:
> error: declaration of 'index' shadows a global declaration [-Wshadow]
>  /usr/include/string.h:489: error: shadowed declaration is here [-Wshadow]
>
>  —
>  You are receiving this because you authored the thread.
>  Reply to this email directly, view it on GitHub
>  <https://github.com/SSSD/sssd/pull/378#issuecomment-328961312>;, or mute
>  the thread
>
> <https://github.com/notifications/unsubscribe-auth/AAG4ek0NUboT94sK1M5Tln3...
>  .
>  
This one is covered by PR #371.

In case @lslebodn agrees I can merge PR #371 and PR #377 and then rebase
this PR atop of that one.

"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-328966393

[sssd PR#378][synchronized] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
@jhrozek, in order to keep this PR moving ... I've decided to do not warn in case of
stack-protection.
I'd like to hear from you whether you want a bug filled for this (as it's
happening in a few pieces of our code).

I've submit the code to our internal CI and I'll post the link as soon as I get
the results.

This PR is rebased atop of #377.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-330771949

[sssd PR#378][-Changes requested] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

Label: -Changes requested

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
@jhrozek, in order to keep this PR moving ... I've decided to do not warn in case of
stack-protector.
I'd like to hear from you whether you want a bug filled for this (as it's
happening in a few pieces of our code).

I've submit the code to our internal CI and I'll post the link as soon as I get
the results.

This PR is rebased atop of #377.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-330771949

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
CI: http://vm-058-233.${abc}/logs/job/77/86/summary.html
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-330785287

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

pbrezina commented:
"""
@fidencio  I'm not Jakub but just file a ticket for those warnings and lets move on
with this one. It's been opened a year ago, you deserve it :-)
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-331397433

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

jhrozek commented:
"""
@fidencio I'm sorry this stalled, but I guess the PR needs a rebase atop master,
right? At least the fixes for the warnings are present in master now..
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-332622358

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
Also, I'd like to open at least one bug to fix some of the warnings we've been
ignoring so far.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-332653890

[sssd PR#378][synchronized] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
@pbrezina, @jhrozek: This PR has been updated and is read to be reviewed & possibly
merged.

I've opened two issues related to this PR: https://pagure.io/SSSD/sssd/issue/3604 and
https://pagure.io/SSSD/sssd/issue/3606
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-351739473

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
I've fired a CI build and I'm waiting for its results.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-351739766

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
CI: http://vm-031.abc.idm.lab.eng.brq.redhat.com/logs/job/82/70/summary.html
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-351793009

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
@pbrezina, @jhrozek: This PR has been updated and is ready to be reviewed & possibly
merged.

I've opened two issues related to this PR: https://pagure.io/SSSD/sssd/issue/3604 and
https://pagure.io/SSSD/sssd/issue/3606
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-351739473

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
I've decided to close this PR, there are more important things to be worked
on/reviewed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-380513866

[sssd PR#378][+Rejected] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

Label: +Rejected

[sssd PR#378][closed] [RFC] Use GNULIB's compiler warning code

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/378
Author: fidencio
 Title: #378: [RFC] Use GNULIB's compiler warning code
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/378/head:pr378
git checkout pr378

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

pbrezina commented:
"""
I'm sorry this got stalled, does it need any rebasing? If you rebase it one more time,
I will review it immediately.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-380708865

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

pbrezina commented:
"""
Same for Amit's patch.
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-380708979

[sssd PR#378][comment] [RFC] Use GNULIB's compiler warning code

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/378
Title: #378: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
@pbrezina, thanks for the offer but there's no need, sincerely. :-)

There's more important work to be done/reviewed at the moment and keeping the PR
opened didn't make sense.

In the future, in case someone has interest on having this merged, we can get back and
talk about it. For now, I'd just keep it closed and forget about this!
"""

See the full comment at https://github.com/SSSD/sssd/pull/378#issuecomment-380721627