[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 1/3] error: don't rely on pointer comparison
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RFC 1/3] error: don't rely on pointer comparisons |
Date: |
Tue, 16 Jun 2015 09:03:44 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
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).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature