Re: [SSSD] [PATCH] MONITOR: Fix warning may be used uninitialized
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..