* migration/rdma.c's macro ERROR()
@ 2023-05-05 10:51 Markus Armbruster
2023-05-05 14:26 ` Peter Xu
0 siblings, 1 reply; 2+ messages in thread
From: Markus Armbruster @ 2023-05-05 10:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Juan Quintela, Peter Xu, Leonardo Bras
migration/rdma.c has
/*
* Print and error on both the Monitor and the Log file.
*/
#define ERROR(errp, fmt, ...) \
do { \
fprintf(stderr, "RDMA ERROR: " fmt "\n", ## __VA_ARGS__); \
if (errp && (*(errp) == NULL)) { \
error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); \
} \
} while (0)
This is problematic. The point of error_setg() & friends is detectin
errors from handling them. error.h:
* - Separation of concerns: the function is responsible for detecting
* errors and failing cleanly; handling the error is its caller's
* job. [...]
Reporting the error to stderr violates this principle. Consequences
include
* When the caller reports the error to stderr, it gets reported there
twice, possibly in slightly different form.
* When the caller recovers from the error cleanly without reporting it,
it is reported to stderr anyway, even though it is not actually an
error.
Mind if I kill the macro?
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: migration/rdma.c's macro ERROR()
2023-05-05 10:51 migration/rdma.c's macro ERROR() Markus Armbruster
@ 2023-05-05 14:26 ` Peter Xu
0 siblings, 0 replies; 2+ messages in thread
From: Peter Xu @ 2023-05-05 14:26 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Juan Quintela, Leonardo Bras, Lidong Chen, Li Zhijian
On Fri, May 05, 2023 at 12:51:45PM +0200, Markus Armbruster wrote:
> migration/rdma.c has
>
> /*
> * Print and error on both the Monitor and the Log file.
> */
> #define ERROR(errp, fmt, ...) \
> do { \
> fprintf(stderr, "RDMA ERROR: " fmt "\n", ## __VA_ARGS__); \
> if (errp && (*(errp) == NULL)) { \
> error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); \
> } \
> } while (0)
>
> This is problematic. The point of error_setg() & friends is detectin
> errors from handling them. error.h:
>
> * - Separation of concerns: the function is responsible for detecting
> * errors and failing cleanly; handling the error is its caller's
> * job. [...]
>
> Reporting the error to stderr violates this principle. Consequences
> include
>
> * When the caller reports the error to stderr, it gets reported there
> twice, possibly in slightly different form.
>
> * When the caller recovers from the error cleanly without reporting it,
> it is reported to stderr anyway, even though it is not actually an
> error.
>
> Mind if I kill the macro?
Makes sense to me.
/me copies the most recent (in past two years) active developers on rdma so
they're aware.
--
Peter Xu
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-05-05 14:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-05 10:51 migration/rdma.c's macro ERROR() Markus Armbruster
2023-05-05 14:26 ` Peter Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).