[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 13/17] nbd: Pass local error object pointer to err
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH 13/17] nbd: Pass local error object pointer to error_append_hint() |
Date: |
Tue, 17 Sep 2019 18:26:56 +0200 |
On Tue, 17 Sep 2019 10:15:07 -0500
Eric Blake <address@hidden> wrote:
> On 9/17/19 5:21 AM, Greg Kurz wrote:
> > Ensure that hints are added even if errp is &error_fatal or &error_abort.
> >
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> > nbd/client.c | 24 +++++++++++++-----------
> > nbd/server.c | 7 +++++--
> > 2 files changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/nbd/client.c b/nbd/client.c
> > index b9dc829175f9..c6e6e4046fd5 100644
> > --- a/nbd/client.c
> > +++ b/nbd/client.c
> > @@ -154,6 +154,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc,
> > NBDOptionReply *reply,
> > bool strict, Error **errp)
> > {
> > g_autofree char *msg = NULL;
> > + Error *local_err = NULL;
>
> I'd prefer 'err' here...
>
> >
> > if (!(reply->type & (1 << 31))) {
> > return 1;
> > @@ -161,14 +162,14 @@ static int nbd_handle_reply_err(QIOChannel *ioc,
> > NBDOptionReply *reply,
> >
> > if (reply->length) {
> > if (reply->length > NBD_MAX_BUFFER_SIZE) {
> > - error_setg(errp, "server error %" PRIu32
> > + error_setg(&local_err, "server error %" PRIu32
>
> so that &err doesn't change line lengths.
>
> > case NBD_REP_ERR_SHUTDOWN:
> > - error_setg(errp, "Server shutting down before option %" PRIu32 "
> > (%s)",
> > + error_setg(&local_err, "Server shutting down before option %"
> > PRIu32 " (%s)",
>
> For example, here, you went beyond 80 columns.
>
> At any rate, I'm assuming this will probably go in through Markus' error
> tree as part of the whole series, rather than me needing to pick just
> this patch through my NBD tree.
>
> Whether or not you shorten the local variable name,
>
Regardless of which tree this goes through, it will be code for
which you're the official maintainer in the end. I'll gladly fix
it to meet your needs :)
> Reviewed-by: Eric Blake <address@hidden>
>