Re: [Qemu-devel] [PATCH v3 3/4] Add Error **errp for xen_pt_config_init()

[Cao jin <caoj.fnst@cn.fujitsu.com>]

On 01/07/2016 12:02 AM, Eric Blake wrote:
> On 01/05/2016 07:39 PM, Cao jin wrote:
[...]
>> +static void xen_pt_config_reg_init(XenPCIPassthroughState *s,
>> +                                  XenPTRegGroup *reg_grp, XenPTRegInfo *reg,
>> +                                  Error **errp)
>
> Indentation is now off.
>

Sharp eyes;)

>> @@ -1967,10 +1970,10 @@ static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
>>               val = data;
>>
>>           if (val & ~size_mask) {
>> -            XEN_PT_ERR(&s->dev,"Offset 0x%04x:0x%04x expands past register size(%d)!\n",
>> -                       offset, val, reg->size);
>> +            error_setg(errp, "Offset 0x%04x:0x%04x expands past"
>> +                    " register size(%d)!", offset, val, reg->size);
>
> Drop the trailing !.  Also, while touching this, it's better to have a
> space before ( in English.
>

Ok

>
>> +void xen_pt_config_init(XenPCIPassthroughState *s, Error **errp)
>>   {
>>       int i, rc;
>> +    Error *local_err = NULL;
>
> Same comments as earlier in the series about using the shorter 'err'
> instead of 'local_err'.
>
>>
>>       QLIST_INIT(&s->reg_grps);
>>
>> @@ -2039,11 +2041,12 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
>>                                                     reg_grp_offset,
>>                                                     &reg_grp_entry->size);
>>               if (rc < 0) {
>> -                XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld, type=0x%x, rc:%d\n",
>> -                           i, ARRAY_SIZE(xen_pt_emu_reg_grps),
>> +                error_setg(&local_err, "Failed to initialize %d/%ld, type=0x%x,"
>> +                           " rc:%d", i, ARRAY_SIZE(xen_pt_emu_reg_grps),
>
> This maps ARRAY_SIZE() (which is size_t) to %ld, which can fail to
> compile on 32-bit platforms (where size_t is not necessarily long).  Fix
> it to %zd while touching it.
>

a question:
1. Is %zu more suitable for size_t? since size_t is unsigned integer.

and a personal question after digging into size_t:
2. Does the size of size_t always equal to the word length[*] of computer

[*] https://en.wikipedia.org/wiki/Word_%28computer_architecture%29

-- 
Yours Sincerely,

Cao jin