[systemd-devel] [PATCH 2/5] libsystemd:terminal :fix uninitialized warning

[Tom Gundersen]

On Mon, Nov 3, 2014 at 1:12 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
> Hi
>
> On Thu, Oct 16, 2014 at 11:43 PM,  <philippedeswert at gmail.com> wrote:
>> From: Philippe De Swert <philippedeswert at gmail.com>
>>
>> Remove the following warning during the compilation:
>> src/libsystemd-terminal/grdev-drm.c: In function 'grdrm_card_hotplug':
>> src/libsystemd-terminal/grdev-drm.c:1087:45: warning: 'fb' may be used uninitialized in this function [-Wmaybe-uninitialized]
>> src/libsystemd-terminal/grdev-drm.c:1035:19: note: 'fb' was declared here
>> ---
>>  src/libsystemd-terminal/grdev-drm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/libsystemd-terminal/grdev-drm.c b/src/libsystemd-terminal/grdev-drm.c
>> index dba6db2..415755e 100644
>> --- a/src/libsystemd-terminal/grdev-drm.c
>> +++ b/src/libsystemd-terminal/grdev-drm.c
>> @@ -1032,7 +1032,7 @@ error:
>>
>>  static void grdrm_crtc_expose(grdrm_crtc *crtc) {
>>          grdrm_pipe *pipe;
>> -        grdrm_fb *fb;
>> +        grdrm_fb *fb  = NULL;
>
> Ewww, this is not nice. It does fix the warning, indeed, but the
> underlying problem is more generic. Lets look at this:
>
> int some_function(void **out) {
> ...
>         r = ioctl(...);
>         if (r < 0)
>                 return -errno;
> ...
> }
>
> gcc has no guarantee that "errno" is <0 if r is <0. Therefore,
> whenever it inlines those functions (-O2 etc.), it will spew a warning
> that "out" might be uninitialized if r<0 but errno==0. With -O0 gcc
> doesn't complain as it probably does not optimize across functions.
> However, with -O2 I get those warnings for "static" functions all the
> time.
>
> Not sure what to do here. I dislike initializing the pointer to NULL
> as it might hide other real warnings. I'd prefer something like:
>
> r = -SANE_ERRNO;
>
> with:
>
> #define SANE_ERRNO (abs(errno) ? : EMAGIC)

Hm, so if somehow errno == 0, we get some bogus errno?

> ...not sure what we do in other places, though. Lennart? Tom?

Will an assert do the trick? Something like

#define NERRNO (assert(errno > 0), -errno)

Then we can replace every

r = foo();
if (r < 0)
    return -errno;

with

r = foo();
if (r < 0)
    return NERRNO;

-t