[systemd-devel] [PATCH] Fix a few compiler warnings

[Tom Gundersen]

On Fri, May 23, 2014 at 11:25 PM, Djalal Harouni <tixxdz at opendz.org> wrote:
> On Wed, May 21, 2014 at 10:46:21AM +0200, Lennart Poettering wrote:
>> On Wed, 21.05.14 10:29, Thomas H.P. Andersen (phomes at gmail.com) wrote:
>>
>> >
>> > On Tue, May 20, 2014 at 5:43 PM, Lennart Poettering
>> > <lennart at poettering.net> wrote:
>> > > On Mon, 19.05.14 19:52, Tom Gundersen (teg at jklm.no) wrote:
>> > >
>> > >> >  _public_ int sd_peer_get_session(int fd, char **session) {
>> > >> > -        struct ucred ucred;
>> > >> > +        struct ucred ucred = {};
>> > >>
>> > >> I can't reproduce this warning, but more importantly, why is this
>> > >> necessary in this function and not the subsequent noes (which all seem
>> > >> to be more or less equivalent)?
>> > >
>> > > Hmm, given the current flakiness of the gcc warnings when -flto is in
>> > > the mix I think we should follow the rule that we do not fix gcc
>> > > warnings that show up only with -flto is used. We can revisit that in a
>> > > few years when LTO has settled a bit, but for now I am pretty sure
>> > > trying to fix all those issues is a waste of time and certainly don't
>> > > improve our code...
>> > >
>> > > Cristian, are those warnings you saw related to -flto?
>> >
>> > The warning in namespace_open is not related to LTO. It shows up with
>> > "autogen.sh g && make" and that gets in the way for my workflow. It
>> > would make my life easier if we could silence it with the fix in this
>> > patch or suppress it with
>> > #pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
>> >
>> > Would that be okay?
>>
>> Ok, fixed that one. It is a false positive, but I can see why gcc gets
>> confused, and this sounds OK to fix. Have done so now. Please test!
> Yes, thank you! I was spammed with reports of namespace_open due to
> optimization flags and later by the LTO on that function each time its
> unit is involved in linking.
>
> There is also this one genrated by LTO, IMO it's a false positive since
> we do *check* for "lease" but the code is not consistent since in that
> code path, "lease" is initialized to NULL in other places, except for
> this one:
>
> src/resolve/resolved-manager.c: In function 'manager_update_resolv_conf':
> src/libsystemd-network/sd-dhcp-lease.c:67:18: warning: 'lease' may be used uninitialized in this function [-Wmaybe-uninitialized]
>         if (lease->dns_size) {
>                  ^
> src/network/sd-network.c:146:24: note: 'lease' was declared here
>         sd_dhcp_lease *lease;
>                        ^
>
> Thanks!

I agree, that's a false positive. However, no harm in fixing it, so
pushed a fix now. Let me know if it is still an issue.

Thanks for reporting!

Tom