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

[Lennart Poettering]

On Mon, 03.11.14 13:12, 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)
> 
> ...not sure what we do in other places, though. Lennart? Tom?

So far we went the simple way out and merged patches like the original
one you are replying to.

Adding a call like this would make sane to me though:

static inline negative_errno(void) {
       return _likely_(errno > 0) ? -errno : -EINVAL;
}

Which is then invoked as:

      return negative_errno();

or so...

Given that this stuff is executed only in the error code, the extra
complexity should not matter I guess. 

I think it would be a mistake to simple negate already negative errno
codes, after all errno should never be negative itself...

Lennart

-- 
Lennart Poettering, Red Hat