[sudo-workers] C99 fixes for the configure script

Florian Weimer fweimer at redhat.com
Wed Apr 26 13:22:23 MDT 2023


* Todd C. Miller:

> On Wed, 26 Apr 2023 11:58:22 +0200, Florian Weimer via sudo-workers wrote:
>
>> We are trying to build Fedora with more C misuse turned into errors:
>>
>>   <https://fedoraproject.org/wiki/Changes/PortingToModernC>
>>   <https://fedoraproject.org/wiki/Toolchain/PortingToModernC>
>
> Great.  I fixed a number of compiler warnings from the configure tests
> several years ago but there are probably more lurking.  Are you running
> configure with "CC=gcc -Werror=implicit-function-declaration" to
> test this?

Unfortunately, that doesn't necessarily detect all issues because the
build still succeeds, potentially changed in unexpected ways.  Others
have config.log diffing to detect that, but I use a patched GCC that
logs these new errors to a magic /usr/lib/gcc/errors/ directory.  If
that directory contains files, the build fails and needs to be inspected
manually.  And -Wno-implicit-function-declaration do not disable the
errors (they are not warnings anymore).  That last part is unfortunately
necessary because some developers just disable warnings instead of
fixing things properly.

There are a few more details (aren't there always?), but that's most of it.

Oh, and we are targing -Wimplicit-int as well during this round (no
issues in sudo found).  The Clang folks are also going after
-Wint-conversion (which is rather misnamed), and a subset of
-Wincompatible-pointer-types.  In current Clang, these are still
warnings, but treated as errors by default.

>> The first issue is a bit tricky.  The lber.h probe also calso ldap_init,
>> but it's deprecated in our <ldap.h> header and only declared if
>> LDAP_DEPRECATED is defined.  So with a C99 compiler without implicit
>> function declaration support, this probe checks for a declaration
>> ldap_init, and not just for <ldap.h> usability without <lber.h>.
>
> Thanks, I've committed a change to just call ldap_msgfree(NULL).

> I committed the change to include stdio.h.

Thanks!

Florian



More information about the sudo-workers mailing list