Re: [Qemu-devel] [PATCH RFC 1/3] error: don't rely on pointer comparisons

[Michael S. Tsirkin]

On Tue, Jun 16, 2015 at 09:03:44AM -0600, Eric Blake wrote:
> On 06/16/2015 06:53 AM, Michael S. Tsirkin wrote:
> > makes it possible to copy error_abort pointers,
> > not just pass them on directly.
> > 
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > ---
> >  util/error.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/util/error.c b/util/error.c
> > index 14f4351..ccf29ea 100644
> > --- a/util/error.c
> > +++ b/util/error.c
> > @@ -20,7 +20,13 @@ struct Error
> >      ErrorClass err_class;
> >  };
> >  
> > -Error *error_abort;
> > +static Error error_abort_st = { .err_class = ERROR_CLASS_MAX };
> > +Error *error_abort = &error_abort_st;
> 
> Looking at this a bit further, I still wonder if we can do a slightly
> better job of coming up with something that will SIGSEGV (or SIGBUS) if
> we (accidentally) try to dereference the pointer (similar to how SIG_IGN
> is (sighandler_t)1) - because we know that the abort object should never
> be dereferenced.  Something like:
> 
>     Error *error_abort = (Error *)1;
> 
> with no need for error_abort_st.  (Might have to spell it as Error
>     *error_abort = (void*)(intptr_t)1;
> to shut up compiler warnings)
> 
> > +
> > +static bool error_is_abort(Error **errp)
> > +{
> > +    return errp && *errp && (*errp)->err_class == ERROR_CLASS_MAX;
> > +}
> 
> and this would be:
> 
>     return errp && *errp == error_abort;
> 
> The rest of this patch is still good.  Then in patch 2, you'd have:
> 
> Error *error_init_local(Error **errp)
> {
>     return error_is_abort(errp) ? error_abort : NULL;
> }
> 
> That is, you still use pointer equality, but at one less level of
> indirection (equality at the Error* level, not the Error** level).

It's a clever trick, it'd work.  But why do tricks? This is never
performance-critical, is it? E.g. debugging is easier if pointers
actually point to things.

Let's see what do others say.


> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>