[libvirt] [PATCH] util: portably check for unchanged uid

[Eric Blake]

On 03/15/2013 10:47 AM, Laine Stump wrote:
> On 03/14/2013 06:35 PM, Eric Blake wrote:
>> We've already scrubbed for comparisons of 'uid_t == -1' (which fail
>> on platforms where uid_t is a u16), but another one snuck in.
> 
> ACK (and sorry for the botched "bugfix" :-/)
> 

>> +# Don't compare *id_t against raw -1.
>> +sc_prohibit_risky_id_promotion:
>> +	@prohibit='\b(user|group|[ug]id) *[=!]= *-'			\
>> +	halt='cast -1 to ([ug]id_t) before comparing against id'	\
>> +	  $(_sc_search_regexp)
>> +
> 
> As we discussed on IRC, I'm slightly concerned about false positives
> when user, group, or [gu]id is used as something other than [gu]id_t,
> but the most common case of this would be for a char*, and I doubt we
> would ever be comparing a char* with !=, 

Well, there's always 'group == NULL' or 'group != NULL'.  But neither of
those patterns match the '-' of '-1' on the other side of the
comparison, so they won't flag as a syntax error.

>so I think it's okay (certainly
> better than the alternative of pushing a release only to find that we
> don't compile on some platform).

Actually, it's worse than not compiling - it's a silent difference in
behavior.  Omitting the cast has a valid compilation regardless of the
size of uid_t, it's just that the rules for how C99 requires it to
behave are not intuitive to a naive programmer who doesn't realize that
the size of uid_t plays a role in what behavior results, and that the
behavior is different for 32-bit vs. 16-bit types.  If we could actually
make it a compiler error (such as with a -Wfoo -Werror combination),
that would be nicer than a syntax check, but as far as I know, gcc
doesn't have anything to help us out on that front.

One thing I did not check for is yoda-isms like '-1 == uid'; but as we
already actively discourage that coding style, I don't think we have to
worry about missing any of those violations.

Thanks for the review, and pushed.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130315/67051b4f/attachment-0001.sig>