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

[Cao jin]

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