Re: [Qemu-ppc] [PATCH 13/17] nbd: Pass local error object pointer to error_append_hint()

[Greg Kurz]

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