On 15.09.25 21:29, Daniel P. Berrangé wrote:
On Mon, Sep 15, 2025 at 09:14:18PM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 15.09.25 18:45, Peter Xu wrote:
On Mon, Sep 15, 2025 at 04:22:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Add a pair to &error_warn helper, to reduce the pattern like
Error *local_err = NULL;
...
if (!foo(..., &local_err)) {
error_report_err(local_err);
return false;
}
to simply
if (!foo(..., &error_reporter)) {
return false;
}
Of course, for new interfaces, it's better to always support and handle
errp argument. But when have to rework the old ones, it's not always
feasible to convert everything to support/handle errp.
The new helper is used in following commits.
Could we add some explanation of why we need this if we already have
error_warn?
I don't see much difference except error_warn will prepend it with
"warning: ", but that doesn't sound like a real problem..
Yes, seems it's the only difference.
For me it seems strange to call it "warning", when we actually go to failure
branch of the code logic.
Finally, we do have error_report() and warn_report(). Seems not a problem to have
corresponding "magic" variables.
If have only one special error variable to simply report the error, I'd prefer it not
have "warning: " prefix.
I.e. drop error_warn, and keep only error_reporter (or some better name?).
FWIW, this whole debate is liable to be a bit of a can of worms that
will delay your series from getting merged.
Honest
Can I suggest you repost this without &error_reporter usage, and then
have a separate standalone series that proposes error_report, and
converts a handful of files to demonstrate its usage.
Agree.
--
Best regards,
Vladimir
