Am 17.06.2015 um 08:57 hat Michael S. Tsirkin geschrieben: > 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 <[email protected]> > > > --- > > > 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.
This isn't about performance, but about failing for invalid use. If we say that dereferencing error_abort is wrong, then letting it fail fast (with a segfault) actually makes debugging easier than silently accessing garbage and trying to figure out why some code misbehaves only later. Kevin
