[SSSD] [PATCH] MONITOR: Fix warning may be used uninitialized

Jakub Hrozek jhrozek at redhat.com
Fri Oct 24 17:13:02 UTC 2014


On Thu, Oct 23, 2014 at 08:11:00PM +0200, Lukas Slebodnik wrote:
> On (23/10/14 19:26), Michal Židek wrote:
> >On 10/23/2014 07:09 PM, Jakub Hrozek wrote:
> >>On Thu, Oct 23, 2014 at 07:02:56PM +0200, Lukas Slebodnik wrote:
> >>>On (23/10/14 18:38), Jakub Hrozek wrote:
> >>>>On Thu, Oct 23, 2014 at 06:35:07PM +0200, Jakub Hrozek wrote:
> >>>>>On Thu, Oct 23, 2014 at 02:14:03PM +0200, Michal Židek wrote:
> >>>>>>On 10/23/2014 11:36 AM, Lukas Slebodnik wrote:
> >>>>>>>ehlo,
> >>>>>>>
> >>>>>>>src/monitor/monitor.c: In function ‘main’:
> >>>>>>>src/monitor/monitor.c:2953:24: error: ‘monitor’ may be used uninitialized
> >>>>>>>                                in this function [-Werror=maybe-uninitialized]
> >>>>>>>      monitor->is_daemon = !opt_interactive;
> >>>>>>>                         ^
> >>>>>>>cc1: all warnings being treated as errors
> >>>>>>>
> >>>>>>>This warning is caused be change 579e5d4b7a3ca161ea7518b2996905fa22c15995
> >>>>>>>"MONITOR: Allow confdb to be accessed by nonroot user"
> >>>>>>>
> >>>>>>>Patch is attached.
> >>>>>>>
> >>>>>>>LS
> >>>>>>>
> >>>>>
> >>>>>Not sure I understand, under what circumstances can errno be 0 while
> >>>>>chown returns 0?
> >>>>
> >>>>Err, while chown returns -1
> >>>Try to explain it gcc with optimization (-O2), but usually gcc does not lie.
> >>
> >>I know, but this code just makes wonder what's going on even now, I'm
> >>pretty sure I will have no idea a year from now...
> >>
> >>>
> >>>another way how to suppress warning is to initialize variabl monitor to NULL
> >>
> >>That sounds like a more readable option to me.
> >
> >Well... if it really is the case that gcc somehow reorganizes the
> >optimized code in a way, that errno set by chown is not catched in
> >the ret variable, then the initialization on NULL will cause crash
> >(but yes it will suppress the warning). As I said, I think it is
> >false positive, but I am in favour of the first solution, (the
> >one I acked already) because it is safer.
> >o

Is there any way to look at what gcc generates in order to understand
how we can get to the faulty situation?

> Michal, Jakub
> it is your decision. (I don't mind)
> I just want to get rid of this warning.
> 
> LS

I'm all for suppressing the warning. But for some reason, the ternary
operator is not too readable to me..

what about something like:

ret = EINVAL; /* Init ret to suppress gcc warning with high -O level */
if (errno) ret = errno;

Would that be more readable?

Please note I don't like the warning either, I just prefer to only
upstram code that will be readable even in future when we look for some
unrelated bug in that area..



More information about the sssd-devel mailing list