On Mon, Nov 3, 2014 at 1:46 PM, David Herrmann <dh.herrm...@gmail.com> wrote: > On Mon, Nov 3, 2014 at 1:44 PM, Zbigniew Jędrzejewski-Szmek > <zbys...@in.waw.pl> wrote: >> On Mon, Nov 03, 2014 at 01:41:20PM +0100, Lennart Poettering wrote: >>> On Mon, 03.11.14 13:12, David Herrmann (dh.herrm...@gmail.com) wrote: >>> >>> > Hi >>> > >>> > On Thu, Oct 16, 2014 at 11:43 PM, <philippedesw...@gmail.com> wrote: >>> > > From: Philippe De Swert <philippedesw...@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... >> I like the assert more, because having errno <= 0 would be a signficant bug >> in the system, not something that we want to ever hide. So basically this >> is a way to tell the compiler what we already know, and assert is better. > > Indeed. So how about: > > static inline int negative_errno(void) { > assert_return(errno > 0, -EINVAL); > return -errno; > }
Looks good to me. -t _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel