From 68a419cc8485295a1a205276602cb3a01cd2e988 Mon Sep 17 00:00:00 2001 From: William Kent Date: Tue, 27 Nov 2018 16:11:43 -0500 Subject: [PATCH] Make the XPC errors regular dictionaries This was harder than it looked, because I could not include . This header declares the variables I need to define as 'const', and that prevents me from using a constructor function to initialize them. I kinda need to do this because the dictionary data structures are so complex. Also fixed up xpc_release() to avoid freeing a statically allocated object (such as our errors). This would still have been a problem with our old error implementation. --- src/libxpc/xpc_dictionary.c | 14 ----- src/libxpc/xpc_error.c | 101 +++++++++++++++++------------------- src/libxpc/xpc_internal.h | 2 + src/libxpc/xpc_misc.c | 18 ++++--- 4 files changed, 62 insertions(+), 73 deletions(-) diff --git a/src/libxpc/xpc_dictionary.c b/src/libxpc/xpc_dictionary.c index c49d486..0260111 100644 --- a/src/libxpc/xpc_dictionary.c +++ b/src/libxpc/xpc_dictionary.c @@ -365,13 +365,6 @@ xpc_dictionary_get_value(xpc_object_t xdict, const char *key) struct xpc_dict_pair *pair; xo = xdict; - if (xo->xo_xpc_type == _XPC_TYPE_STATIC_ERROR) { - xpc_object_t error = xpc_hydrate_static_error(xdict); - xpc_object_t value = xpc_dictionary_get_value(error, key); - xpc_release(error); - return value; - } - xpc_assert_type(xo, _XPC_TYPE_DICTIONARY); head = &xo->xo_dict; @@ -391,13 +384,6 @@ xpc_dictionary_get_count(xpc_object_t xdict) struct xpc_object *xo; xo = xdict; - if (xo->xo_xpc_type == _XPC_TYPE_STATIC_ERROR) { - xpc_object_t error = xpc_hydrate_static_error(xdict); - size_t count = xpc_dictionary_get_count(error); - xpc_release(error); - return count; - } - xpc_assert_type(xdict, _XPC_TYPE_DICTIONARY); return (xo->xo_size); } diff --git a/src/libxpc/xpc_error.c b/src/libxpc/xpc_error.c index 5805d5d..2242e70 100644 --- a/src/libxpc/xpc_error.c +++ b/src/libxpc/xpc_error.c @@ -1,4 +1,29 @@ -#include +// We cannot include xpc/xpc.h in this file, as otherwise the definitions +// of the _xpc_error_* symbols would not compile correctly. Therefore, I +// must redefine all applicable symbols here. How annoying. + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +typedef struct xpc_object * xpc_object_t; +typedef void (^xpc_handler_t)(xpc_object_t object); + +extern xpc_object_t xpc_string_create(const char *); +extern void xpc_dictionary_set_value(xpc_object_t, const char *, xpc_object_t); +extern void xpc_release(xpc_object_t); + #include "xpc_internal.h" #define xpc_assert_type(xo, type) \ @@ -8,60 +33,30 @@ const char * const _xpc_error_key_description = "XPC_ERROR_DESCRIPTION"; -// N.B.: This type as used in this file *MUST* have the same layout as struct xpc_object! -// I cannot make them identical due to compiler insistance otherwise. struct _xpc_dictionary_s { - uint8_t xo_xpc_type; - uint16_t xo_flags; - volatile uint32_t xo_refcnt; - size_t xo_size; - xpc_u xo_u; - audit_token_t * xo_audit_token; - TAILQ_ENTRY(xpc_object) xo_link; + struct xpc_object object; }; -#define XPC_MAGIC_DICT_INTERRUPTED_ERROR (uint16_t)0x10001 -#define XPC_MAGIC_DICT_INVALID_ERROR (uint16_t)0x10002 -#define XPC_MAGIC_DICT_TERMINATION_IMMINENT (uint16_t)0x10003 - -const struct _xpc_dictionary_s _xpc_error_connection_interrupted = { - _XPC_TYPE_STATIC_ERROR, - XPC_MAGIC_DICT_INTERRUPTED_ERROR -}; -const struct _xpc_dictionary_s _xpc_error_connection_invalid = { - _XPC_TYPE_STATIC_ERROR, - XPC_MAGIC_DICT_INVALID_ERROR -}; -const struct _xpc_dictionary_s _xpc_error_termination_imminent = { - _XPC_TYPE_STATIC_ERROR, - XPC_MAGIC_DICT_TERMINATION_IMMINENT -}; - -xpc_object_t xpc_hydrate_static_error(xpc_object_t input) { - xpc_assert_nonnull(input); - xpc_assert_type(input, _XPC_TYPE_STATIC_ERROR); - - struct xpc_object *xo = input; - - if (xo->xo_flags == XPC_MAGIC_DICT_INVALID_ERROR) { - const char *key = XPC_ERROR_KEY_DESCRIPTION; - xpc_object_t value = xpc_string_create("Connection invalid"); - xpc_object_t result = xpc_dictionary_create(&key, &value, 1); - xpc_release(value); - return result; - } else if (xo->xo_flags == XPC_MAGIC_DICT_INTERRUPTED_ERROR) { - const char *key = XPC_ERROR_KEY_DESCRIPTION; - xpc_object_t value = xpc_string_create("Connection interrupted"); - xpc_object_t result = xpc_dictionary_create(&key, &value, 1); - xpc_release(value); - return result; - } else if (xo->xo_flags == XPC_MAGIC_DICT_TERMINATION_IMMINENT) { - const char *key = XPC_ERROR_KEY_DESCRIPTION; - xpc_object_t value = xpc_string_create("Process termination imminent"); - xpc_object_t result = xpc_dictionary_create(&key, &value, 1); - xpc_release(value); - return result; - } +struct _xpc_dictionary_s _xpc_error_connection_interrupted = {0}; +struct _xpc_dictionary_s _xpc_error_connection_invalid = {0}; +struct _xpc_dictionary_s _xpc_error_termination_imminent = {0}; + +static void _xpc_initialize_error_object(struct xpc_object *object, const char *description) { + object->xo_xpc_type = _XPC_TYPE_DICTIONARY; + object->xo_flags = _XPC_STATIC_OBJECT_FLAG; + object->xo_refcnt = 1; + object->xo_audit_token = NULL; + object->xo_size = 1; + TAILQ_INIT(&object->xo_dict); + + xpc_object_t descriptionObject = xpc_string_create(description); + xpc_dictionary_set_value(object, _xpc_error_key_description, descriptionObject); + xpc_release(descriptionObject); +} - xpc_api_misuse("Invalid XPC magic dict ID %d", xo->xo_flags); +__attribute__((visibility("hidden"))) +void _xpc_initialize_errors(void) { + _xpc_initialize_error_object(&_xpc_error_connection_invalid.object, "Connection invalid"); + _xpc_initialize_error_object(&_xpc_error_connection_interrupted.object, "Connection interrupted"); + _xpc_initialize_error_object(&_xpc_error_termination_imminent.object, "Process termination imminent"); } diff --git a/src/libxpc/xpc_internal.h b/src/libxpc/xpc_internal.h index ee7fd7f..e7dbe15 100644 --- a/src/libxpc/xpc_internal.h +++ b/src/libxpc/xpc_internal.h @@ -87,6 +87,8 @@ typedef union { #define _XPC_FROM_WIRE 0x1 +#define _XPC_STATIC_OBJECT_FLAG 0x2 + struct xpc_object { uint8_t xo_xpc_type; uint16_t xo_flags; diff --git a/src/libxpc/xpc_misc.c b/src/libxpc/xpc_misc.c index eedc24a..b517c81 100644 --- a/src/libxpc/xpc_misc.c +++ b/src/libxpc/xpc_misc.c @@ -155,13 +155,14 @@ void xpc_release(xpc_object_t obj) { struct xpc_object *xo; - xo = obj; -// if (atomic_fetchadd_int(&xo->xo_refcnt, -1) > 1) - // TODO: _sjc_ replace this with eg. stdatomic.s - if ( --(xo->xo_refcnt) > 1 ) { - return; - } + + // Don't destroy statically allocated objects. (These are compiled + // into a binary, and as such we can't call free() on them.) + if ((xo->xo_flags & _XPC_STATIC_OBJECT_FLAG) != 0 && xo->xo_refcnt == 1) + return; + if ( --(xo->xo_refcnt) > 1 ) + return; xpc_object_destroy(xo); } @@ -546,3 +547,8 @@ xpc_call_wakeup(mach_port_t rport, int retcode) return (err); } + +extern void _xpc_initialize_errors(void); +void _libxpc_initializer(void) { + _xpc_initialize_errors(); +}