1. Patchew
  2. Xen
  3. View series

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

Juergen Gross posted 1 patch 2 years, 4 months ago
Test gitlab-ci passed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20211209120939.513-1-jgross@suse.com
There is a newer version of this series
tools/libs/ctrl/xc_private.h | 32 +++++++++++---------------------
1 file changed, 11 insertions(+), 21 deletions(-)
[PATCH] tools/libs/ctrl: Save errno only once in *PRINTF() and *ERROR()
Posted by Juergen Gross 2 years, 4 months ago 
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@suse.com>
---
 tools/libs/ctrl/xc_private.h | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/tools/libs/ctrl/xc_private.h b/tools/libs/ctrl/xc_private.h
index 2e483590e6..a3fe475c75 100644
--- 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))
 
 /*
  * HYPERCALL ARGUMENT BUFFERS
-- 
2.26.2
Re: [PATCH] tools/libs/ctrl: Save errno only once in *PRINTF() and *ERROR()
Posted by Jan Beulich 2 years, 4 months ago 
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@suse.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
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.

Jan
Re: [PATCH] tools/libs/ctrl: Save errno only once in *PRINTF() and *ERROR()
Posted by Juergen Gross 2 years, 4 months ago 
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@suse.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.