diff mbox

[10/13] hw: Don't call visit_end_struct() after visit_start_struct() fails

Message ID 1399034675-17844-11-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster May 2, 2014, 12:44 p.m. UTC 
When visit_start_struct() succeeds, visit_end_struct() must not be
called.  rtc_get_date() and balloon_stats_all() call it anyway.  As
far as I can tell, they're only used with the string output visitor,
which doesn't care.  Fix them anyway.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/timer/mc146818rtc.c     | 23 +++++++++++++++--------
 hw/virtio/virtio-balloon.c | 25 +++++++++++++++++++------
 2 files changed, 34 insertions(+), 14 deletions(-)

Comments

Eric Blake May 5, 2014, 8:56 p.m. UTC | #1 
On 05/02/2014 06:44 AM, Markus Armbruster wrote:
> When visit_start_struct() succeeds, visit_end_struct() must not be

s/succeeds/fails/ (this really confused me on my first read, until I saw
the code and the subject line and determined the typo)

> called.  rtc_get_date() and balloon_stats_all() call it anyway.  As
> far as I can tell, they're only used with the string output visitor,
> which doesn't care.  Fix them anyway.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/timer/mc146818rtc.c     | 23 +++++++++++++++--------
>  hw/virtio/virtio-balloon.c | 25 +++++++++++++++++++------
>  2 files changed, 34 insertions(+), 14 deletions(-)

With commit message fixed,

Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster May 6, 2014, 12:22 p.m. UTC | #2 
Eric Blake <eblake@redhat.com> writes:

> On 05/02/2014 06:44 AM, Markus Armbruster wrote:
>> When visit_start_struct() succeeds, visit_end_struct() must not be
>
> s/succeeds/fails/ (this really confused me on my first read, until I saw
> the code and the subject line and determined the typo)

D'oh!  Will fix both here an in 10/13/

>> called.  rtc_get_date() and balloon_stats_all() call it anyway.  As
>> far as I can tell, they're only used with the string output visitor,
>> which doesn't care.  Fix them anyway.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/timer/mc146818rtc.c     | 23 +++++++++++++++--------
>>  hw/virtio/virtio-balloon.c | 25 +++++++++++++++++++------
>>  2 files changed, 34 insertions(+), 14 deletions(-)
>
> With commit message fixed,
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
diff mbox

Patch

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 8509309..6c3e3b6 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -793,19 +793,26 @@  static const MemoryRegionOps cmos_ops = {
 static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
                          const char *name, Error **errp)
 {
+    Error *err = NULL;
     RTCState *s = MC146818_RTC(obj);
     struct tm current_tm;
 
     rtc_update_time(s);
     rtc_get_time(s, &current_tm);
-    visit_start_struct(v, NULL, "struct tm", name, 0, errp);
-    visit_type_int32(v, &current_tm.tm_year, "tm_year", errp);
-    visit_type_int32(v, &current_tm.tm_mon, "tm_mon", errp);
-    visit_type_int32(v, &current_tm.tm_mday, "tm_mday", errp);
-    visit_type_int32(v, &current_tm.tm_hour, "tm_hour", errp);
-    visit_type_int32(v, &current_tm.tm_min, "tm_min", errp);
-    visit_type_int32(v, &current_tm.tm_sec, "tm_sec", errp);
-    visit_end_struct(v, errp);
+    visit_start_struct(v, NULL, "struct tm", name, 0, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int32(v, &current_tm.tm_year, "tm_year", &err);
+    visit_type_int32(v, &current_tm.tm_mon, "tm_mon", &err);
+    visit_type_int32(v, &current_tm.tm_mday, "tm_mday", &err);
+    visit_type_int32(v, &current_tm.tm_hour, "tm_hour", &err);
+    visit_type_int32(v, &current_tm.tm_min, "tm_min", &err);
+    visit_type_int32(v, &current_tm.tm_sec, "tm_sec", &err);
+    visit_end_struct(v, &err);
+
+out:
+    error_propagate(errp, err);
 }
 
 static void rtc_realizefn(DeviceState *dev, Error **errp)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 51f02eb..65d05c8 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -108,6 +108,7 @@  static void balloon_stats_poll_cb(void *opaque)
 static void balloon_stats_get_all(Object *obj, struct Visitor *v,
                                   void *opaque, const char *name, Error **errp)
 {
+    Error *err = NULL;
     VirtIOBalloon *s = opaque;
     int i;
 
@@ -116,17 +117,29 @@  static void balloon_stats_get_all(Object *obj, struct Visitor *v,
         return;
     }
 
-    visit_start_struct(v, NULL, "guest-stats", name, 0, errp);
-    visit_type_int(v, &s->stats_last_update, "last-update", errp);
+    visit_start_struct(v, NULL, "guest-stats", name, 0, &err);
+    if (err) {
+        goto out;
+    }
+
+    visit_type_int(v, &s->stats_last_update, "last-update", &err);
 
-    visit_start_struct(v, NULL, NULL, "stats", 0, errp);
+    visit_start_struct(v, NULL, NULL, "stats", 0, &err);
+    if (err) {
+        goto out_end;
+    }
+        
     for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
         visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
-                         errp);
+                         &err);
     }
-    visit_end_struct(v, errp);
+    visit_end_struct(v, &err);
+
+out_end:
+    visit_end_struct(v, &err);
 
-    visit_end_struct(v, errp);
+out:
+    error_propagate(errp, err);
 }
 
 static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,