Re: [PATCH] tools/libs/ctrl: Save errno only once in *PRINTF() and *ERROR()

[Juergen Gross <jgross@xxxxxxxx>]

On 09.12.21 14:31, Jan Beulich wrote:
> On 09.12.2021 13:09, Juergen Gross wrote:
> > All *PRINTF() and *ERROR() macros are based on xc_reportv() which is
> > saving and restoring errno in order to not modify it. There is no need
> > to save and restore in those macros, too.
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> Albeit ...
>
> > --- a/tools/libs/ctrl/xc_private.h
> > +++ b/tools/libs/ctrl/xc_private.h
> > @@ -122,28 +122,18 @@ void xc_report_progress_step(xc_interface *xch,
> >   
> >   /* anamorphic macros:  struct xc_interface *xch  must be in scope */
> >   
> > -#define IPRINTF(_f, _a...)  do { int IPRINTF_errno = errno; \
> > -        xc_report(xch, xch->error_handler, XTL_INFO,0, _f , ## _a); \
> > -        errno = IPRINTF_errno; \
> > -        } while (0)
> > -#define DPRINTF(_f, _a...) do { int DPRINTF_errno = errno; \
> > -        xc_report(xch, xch->error_handler, XTL_DETAIL,0, _f , ## _a); \
> > -        errno = DPRINTF_errno; \
> > -        } while (0)
> > -#define DBGPRINTF(_f, _a...)  do { int DBGPRINTF_errno = errno; \
> > -        xc_report(xch, xch->error_handler, XTL_DEBUG,0, _f , ## _a); \
> > -        errno = DBGPRINTF_errno; \
> > -        } while (0)
> > -
> > -#define ERROR(_m, _a...)  do { int ERROR_errno = errno; \
> > -        xc_report_error(xch,XC_INTERNAL_ERROR,_m , ## _a ); \
> > -        errno = ERROR_errno; \
> > -        } while (0)
> > -#define PERROR(_m, _a...) do { int PERROR_errno = errno; \
> > +#define IPRINTF(_f, _a...) \
> > +        xc_report(xch, xch->error_handler, XTL_INFO,0, _f , ## _a)
> > +#define DPRINTF(_f, _a...) \
> > +        xc_report(xch, xch->error_handler, XTL_DETAIL,0, _f , ## _a)
> > +#define DBGPRINTF(_f, _a...) \
> > +        xc_report(xch, xch->error_handler, XTL_DEBUG,0, _f , ## _a)
> > +
> > +#define ERROR(_m, _a...) \
> > +        xc_report_error(xch,XC_INTERNAL_ERROR,_m , ## _a )
> > +#define PERROR(_m, _a...) \
> >           xc_report_error(xch,XC_INTERNAL_ERROR,_m " (%d = %s)", \
> > -        ## _a , errno, xc_strerror(xch, errno)); \
> > -        errno = PERROR_errno; \
> > -        } while (0)
> > +                        ## _a , errno, xc_strerror(xch, errno))
> ... while I realize you only stripped only semicolons and line
> continuations, but I would find it quite desirable to also get the
> use of blanks straightened at this occasion: In a number of cases
> commas aren't followed by blanks and (instead?) sometimes are
> preceded by ones. It doesn't seem very likely to me that this
> would be intentional.
Let me resend it with proper style corrections.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature