| Message ID | 20180830145902.27376-3-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qemu-error: advanced report_once handling | expand |
Cornelia Huck <cohuck@redhat.com> writes: > {error,warn}_report_once() are a special case of the new functions > and can simply switch to them. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > include/qemu/error-report.h | 34 ++++++++++++++-------------------- > 1 file changed, 14 insertions(+), 20 deletions(-) > > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h > index e415128ac4..918cb936d8 100644 > --- a/include/qemu/error-report.h > +++ b/include/qemu/error-report.h > @@ -53,32 +53,26 @@ bool warn_report_once_cond(bool *printed, const char *fmt, ...) > * Similar to error_report(), except it prints the message just once. > * Return true when it prints, false otherwise. > */ > -#define error_report_once(fmt, ...) \ > - ({ \ > - static bool print_once_; \ > - bool ret_print_once_ = !print_once_; \ > - \ > - if (!print_once_) { \ > - print_once_ = true; \ > - error_report(fmt, ##__VA_ARGS__); \ > - } \ > - unlikely(ret_print_once_); \ > +#define error_report_once(fmt, ...) \ > + ({ \ > + static bool print_once_; \ > + bool ret_print_once_ = \ > + error_report_once_cond(&print_once_, \ > + fmt, ##__VA_ARGS__); \ > + unlikely(ret_print_once_); \ > }) Do we still need @ret_print_once_? #define error_report_once(fmt, ...) \ ({ \ static bool print_once_; \ unlikely(error_report_once_cond(&print_once_, \ fmt, ##__VA_ARGS__)); \ }) Or dispense with the unlikely: #define error_report_once(fmt, ...) \ ({ \ static bool print_once_; \ error_report_once_cond(&print_once_, \ fmt, ##__VA_ARGS__); \ }) Got a preference? > > /* > * Similar to warn_report(), except it prints the message just once. > * Return true when it prints, false otherwise. > */ > -#define warn_report_once(fmt, ...) \ > - ({ \ > - static bool print_once_; \ > - bool ret_print_once_ = !print_once_; \ > - \ > - if (!print_once_) { \ > - print_once_ = true; \ > - warn_report(fmt, ##__VA_ARGS__); \ > - } \ > - unlikely(ret_print_once_); \ > +#define warn_report_once(fmt, ...) \ > + ({ \ > + static bool print_once_; \ > + bool ret_print_once_ = \ > + warn_report_once_cond(&print_once_, \ > + fmt, ##__VA_ARGS__); \ > + unlikely(ret_print_once_); \ > }) > > const char *error_get_progname(void);
On Fri, 31 Aug 2018 08:01:39 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Cornelia Huck <cohuck@redhat.com> writes: > > > {error,warn}_report_once() are a special case of the new functions > > and can simply switch to them. > > > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > include/qemu/error-report.h | 34 ++++++++++++++-------------------- > > 1 file changed, 14 insertions(+), 20 deletions(-) > > > > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h > > index e415128ac4..918cb936d8 100644 > > --- a/include/qemu/error-report.h > > +++ b/include/qemu/error-report.h > > @@ -53,32 +53,26 @@ bool warn_report_once_cond(bool *printed, const char *fmt, ...) > > * Similar to error_report(), except it prints the message just once. > > * Return true when it prints, false otherwise. > > */ > > -#define error_report_once(fmt, ...) \ > > - ({ \ > > - static bool print_once_; \ > > - bool ret_print_once_ = !print_once_; \ > > - \ > > - if (!print_once_) { \ > > - print_once_ = true; \ > > - error_report(fmt, ##__VA_ARGS__); \ > > - } \ > > - unlikely(ret_print_once_); \ > > +#define error_report_once(fmt, ...) \ > > + ({ \ > > + static bool print_once_; \ > > + bool ret_print_once_ = \ > > + error_report_once_cond(&print_once_, \ > > + fmt, ##__VA_ARGS__); \ > > + unlikely(ret_print_once_); \ > > }) > > Do we still need @ret_print_once_? > > #define error_report_once(fmt, ...) \ > ({ \ > static bool print_once_; \ > unlikely(error_report_once_cond(&print_once_, \ > fmt, ##__VA_ARGS__)); \ > }) > > Or dispense with the unlikely: > > #define error_report_once(fmt, ...) \ > ({ \ > static bool print_once_; \ > error_report_once_cond(&print_once_, \ > fmt, ##__VA_ARGS__); \ > }) > > Got a preference? I think we can get rid of the unlikely(). Will you make these changes yourself, or should I respin?
Cornelia Huck <cohuck@redhat.com> writes: > On Fri, 31 Aug 2018 08:01:39 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> Cornelia Huck <cohuck@redhat.com> writes: >> >> > {error,warn}_report_once() are a special case of the new functions >> > and can simply switch to them. >> > >> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> > --- >> > include/qemu/error-report.h | 34 ++++++++++++++-------------------- >> > 1 file changed, 14 insertions(+), 20 deletions(-) >> > >> > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h >> > index e415128ac4..918cb936d8 100644 >> > --- a/include/qemu/error-report.h >> > +++ b/include/qemu/error-report.h >> > @@ -53,32 +53,26 @@ bool warn_report_once_cond(bool *printed, const char *fmt, ...) >> > * Similar to error_report(), except it prints the message just once. >> > * Return true when it prints, false otherwise. >> > */ >> > -#define error_report_once(fmt, ...) \ >> > - ({ \ >> > - static bool print_once_; \ >> > - bool ret_print_once_ = !print_once_; \ >> > - \ >> > - if (!print_once_) { \ >> > - print_once_ = true; \ >> > - error_report(fmt, ##__VA_ARGS__); \ >> > - } \ >> > - unlikely(ret_print_once_); \ >> > +#define error_report_once(fmt, ...) \ >> > + ({ \ >> > + static bool print_once_; \ >> > + bool ret_print_once_ = \ >> > + error_report_once_cond(&print_once_, \ >> > + fmt, ##__VA_ARGS__); \ >> > + unlikely(ret_print_once_); \ >> > }) >> >> Do we still need @ret_print_once_? >> >> #define error_report_once(fmt, ...) \ >> ({ \ >> static bool print_once_; \ >> unlikely(error_report_once_cond(&print_once_, \ >> fmt, ##__VA_ARGS__)); \ >> }) >> >> Or dispense with the unlikely: >> >> #define error_report_once(fmt, ...) \ >> ({ \ >> static bool print_once_; \ >> error_report_once_cond(&print_once_, \ >> fmt, ##__VA_ARGS__); \ >> }) >> >> Got a preference? > > I think we can get rid of the unlikely(). With that: Reviewed-by: Markus Armbruster <armbru@redhat.com> > Will you make these changes yourself, or should I respin? Happy to do it myself.
diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index e415128ac4..918cb936d8 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -53,32 +53,26 @@ bool warn_report_once_cond(bool *printed, const char *fmt, ...) * Similar to error_report(), except it prints the message just once. * Return true when it prints, false otherwise. */ -#define error_report_once(fmt, ...) \ - ({ \ - static bool print_once_; \ - bool ret_print_once_ = !print_once_; \ - \ - if (!print_once_) { \ - print_once_ = true; \ - error_report(fmt, ##__VA_ARGS__); \ - } \ - unlikely(ret_print_once_); \ +#define error_report_once(fmt, ...) \ + ({ \ + static bool print_once_; \ + bool ret_print_once_ = \ + error_report_once_cond(&print_once_, \ + fmt, ##__VA_ARGS__); \ + unlikely(ret_print_once_); \ }) /* * Similar to warn_report(), except it prints the message just once. * Return true when it prints, false otherwise. */ -#define warn_report_once(fmt, ...) \ - ({ \ - static bool print_once_; \ - bool ret_print_once_ = !print_once_; \ - \ - if (!print_once_) { \ - print_once_ = true; \ - warn_report(fmt, ##__VA_ARGS__); \ - } \ - unlikely(ret_print_once_); \ +#define warn_report_once(fmt, ...) \ + ({ \ + static bool print_once_; \ + bool ret_print_once_ = \ + warn_report_once_cond(&print_once_, \ + fmt, ##__VA_ARGS__); \ + unlikely(ret_print_once_); \ }) const char *error_get_progname(void);
{error,warn}_report_once() are a special case of the new functions and can simply switch to them. Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- include/qemu/error-report.h | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-)