[libvirt] [PATCH 12/12] storage_conf: Use uid_t/gid_t instead of int to cast the value

[Osier Yang]

On 24/05/13 22:11, Eric Blake wrote:
> On 05/22/2013 06:05 AM, Osier Yang wrote:
>> And error out if the casted value is not same with the original
>> one, which prevents the bug on platform(s) where uid_t/gid_t
>> has different size with long.
>> ---
>>   src/conf/storage_conf.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>>       } else {
>> -        if (virXPathLong("number(./owner)", ctxt, &v) < 0) {
>> +        if (virXPathLong("number(./owner)", ctxt, &val) < 0 ||
>> +            (uid_t)val != val) {
> Once you have made this check...
>
>>               virReportError(VIR_ERR_XML_ERROR, "%s",
>>                              _("malformed owner element"));
>>               goto error;
>>           }
>> -        perms->uid = (int)v;
>> +
>> +        perms->uid = (uid_t)val;
> ...the cast here is redundant.  You could write 'perms->uid = val'.
>
>>       }
>>   
>>       if (virXPathNode("./group", ctxt) == NULL) {
>>           perms->gid = (gid_t) -1;
>>       } else {
>> -        if (virXPathLong("number(./group)", ctxt, &v) < 0) {
>> +        if (virXPathLong("number(./group)", ctxt, &val) < 0 ||
>> +            (gid_t)val != val) {
>>               virReportError(VIR_ERR_XML_ERROR, "%s",
>>                              _("malformed group element"));
>>               goto error;
>>           }
>> -        perms->gid = (int)v;
>> +        perms->gid = (gid_t)val;
> Likewise.
>
> ACK with that simplification, and with your followup that explicitly
> allows -1.
>

Thanks for the reviewing, I pushed the left patches (except 10/12) with the
small nits pointed by you guys fixed.

Osier