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

[Eric Blake]

On 01/05/2016 07:39 PM, Cao jin wrote:
> To catch the error msg. Also modify the caller
> 
> Signed-off-by: Cao jin <address@hidden>
> ---
>  hw/xen/xen_pt.c             |  7 ++++---
>  hw/xen/xen_pt.h             |  2 +-
>  hw/xen/xen_pt_config_init.c | 51 
> ++++++++++++++++++++++++---------------------
>  3 files changed, 32 insertions(+), 28 deletions(-)
> 

> +++ b/hw/xen/xen_pt_config_init.c
> @@ -1899,8 +1899,9 @@ static uint8_t find_cap_offset(XenPCIPassthroughState 
> *s, uint8_t cap)
>      return 0;
>  }
>  
> -static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
> -                                  XenPTRegGroup *reg_grp, XenPTRegInfo *reg)
> +static void xen_pt_config_reg_init(XenPCIPassthroughState *s,
> +                                  XenPTRegGroup *reg_grp, XenPTRegInfo *reg,
> +                                  Error **errp)

Indentation is now off.


> @@ -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.


> +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.

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

signature.asc
Description: OpenPGP digital signature