[PATCH] dbus-vmstate: replace g_return with error handling

[PATCH] dbus-vmstate: replace g_return with error handling

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since g_input_stream_read_all() may return less than requested when the
stream is malformed, we should treat this condition as a runtime user
error (g_return are for programming errors).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 backends/dbus-vmstate.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
index bd050e8e9c..616d291cfb 100644
--- a/backends/dbus-vmstate.c
+++ b/backends/dbus-vmstate.c
@@ -229,7 +229,10 @@ static int dbus_vmstate_post_load(void *opaque, int version_id)
                                      &bytes_read, NULL, &err)) {
             goto error;
         }
-        g_return_val_if_fail(bytes_read == len, -1);
+        if (bytes_read != len) {
+            error_report("%s: Failed to read proxy Id", __func__);
+            return -1;
+        }
         id[len] = 0;
 
         trace_dbus_vmstate_loading(id);
-- 
2.29.0

Re: [PATCH] dbus-vmstate: replace g_return with error handling

[Markus Armbruster]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since g_input_stream_read_all() may return less than requested when the
> stream is malformed, we should treat this condition as a runtime user
> error (g_return are for programming errors).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  backends/dbus-vmstate.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
> index bd050e8e9c..616d291cfb 100644
> --- a/backends/dbus-vmstate.c
> +++ b/backends/dbus-vmstate.c
> @@ -229,7 +229,10 @@ static int dbus_vmstate_post_load(void *opaque, int version_id)
>                                       &bytes_read, NULL, &err)) {
>              goto error;
>          }
> -        g_return_val_if_fail(bytes_read == len, -1);
> +        if (bytes_read != len) {
> +            error_report("%s: Failed to read proxy Id", __func__);

Error messages containing function names are code smell.  It's
consustent with nearby errors, i.e. this patch is not to blame.

> +            return -1;
> +        }
>          id[len] = 0;
>  
>          trace_dbus_vmstate_loading(id);

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH] dbus-vmstate: replace g_return with error handling

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

Hi

On Wed, Nov 18, 2020 at 8:33 PM Markus Armbruster <armbru@redhat.com> wrote:

> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Since g_input_stream_read_all() may return less than requested when the
> > stream is malformed, we should treat this condition as a runtime user
> > error (g_return are for programming errors).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  backends/dbus-vmstate.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
> > index bd050e8e9c..616d291cfb 100644
> > --- a/backends/dbus-vmstate.c
> > +++ b/backends/dbus-vmstate.c
> > @@ -229,7 +229,10 @@ static int dbus_vmstate_post_load(void *opaque, int
> version_id)
> >                                       &bytes_read, NULL, &err)) {
> >              goto error;
> >          }
> > -        g_return_val_if_fail(bytes_read == len, -1);
> > +        if (bytes_read != len) {
> > +            error_report("%s: Failed to read proxy Id", __func__);
>
> Error messages containing function names are code smell.  It's
> consustent with nearby errors, i.e. this patch is not to blame.
>
> > +            return -1;
> > +        }
> >          id[len] = 0;
> >
> >          trace_dbus_vmstate_loading(id);
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
>
>
For the record, Markus sent a similar patch later "backends/dbus-vmstate:
Fix short read error handling", which has been applied.

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2526 bytes --]