On 18.07.2013 15:16, Eric Blake wrote:
> On 07/18/2013 06:46 AM, Michal Privoznik wrote:
>> On 18.07.2013 01:08, Eric Blake wrote:
>>> Commit 75c1256 states that virGetGroupList must not be called
>>> between fork and exec, then commit ee777e99 promptly violated
>>> that for lxc's use of virSecurityManagerSetProcessLabel. Hoist
>>> the supplemental group detection to the time that the security
>>> manager is created. Qemu is safe, as it uses
>>> virSecurityManagerSetChildProcessLabel which in turn uses
>>> virCommand to determine supplemental groups.
>>>
>
>>> - if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0)
>>> + if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) {
>>> + if (groups)
>>> + *groups = NULL;
>>> + if (ngroups)
>>> + ngroups = 0;
>>
>> I believe you wanted *ngroups = 0; in here.
>>
>
> Indeed. I blame C for treating 0 and NULL interchangeably.
>
>>
>> ACK series, but see the issue I'm raising in 2/2.
>
> Thanks; I'll push after fixing that typo.
>
In fact, gcc warned me about possibly unused variable:
security/security_dac.c: In function 'virSecurityDACSetProcessLabel':
security/security_dac.c:1038:21: error: 'ngroups' may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (virSetUIDGID(user, group, groups, ngroups) < 0)
^
security/security_dac.c: At top level:
cc1: error: unrecognized command line option "-Wno-unused-command-line-argument" [-Werror]
cc1: all warnings being treated as errors
Truth is, I'm using the most recent gcc (4.8.1). Which is both advantage and disadvantage. One hand, I can catch errors like these, on the other hand, the gcc bug in static analysis has been fixed. The bug, where local variable 'shadows' global function. This is NOT what I call shadowing.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list