Re: [PATCH v3 04/13] util/error: add &error_reporter

["Daniel P. Berrangé" <berrange@redhat.com>]

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.

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.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|