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