diff options
| -rw-r--r-- | ChangeLog | 25 | ||||
| -rw-r--r-- | bus/activation.c | 71 | ||||
| -rw-r--r-- | bus/config-loader-libxml.c | 265 | ||||
| -rw-r--r-- | configure.in | 5 | ||||
| -rw-r--r-- | dbus/dbus-message.c | 18 | ||||
| -rw-r--r-- | dbus/dbus-transport-unix.c | 24 | ||||
| -rw-r--r-- | glib/Makefile.am | 2 | ||||
| -rw-r--r-- | test/data/invalid-config-files/not-well-formed.conf | 5 | ||||
| -rw-r--r-- | test/data/invalid-config-files/truncated-file.conf | 9 |
9 files changed, 310 insertions, 114 deletions
@@ -1,3 +1,28 @@ +2004-07-29 Olivier Andrieu <oliv__a@users.sourceforge.net> + + * bus/config-loader-libxml.c: complete the implementation of + libxml backend for config file loader. Doesn't work with full OOM + test yet. + + * configure.in: change error when selecting libxml into a warning. + + * test/data/invalid-config-files: add two non-well-formed XML + files. + + * glib/Makefile.am: libdbus_gtool always uses expat, not libxml. + + * dbus/dbus-transport-unix.c (unix_handle_watch): do not + disconnect in case of DBUS_WATCH_HANGUP, several do_reading() may + be necessary to read all the buffer. (bug #894) + + * bus/activation.c (bus_activation_activate_service): fix a + potential assertion failure (bug #896). Small optimization in the + case of auto-activation messages. + + * dbus/dbus-message.c (verify_test_message, _dbus_message_test): + add test case for byte-through-vararg bug (#901). patch by Kimmo + Hämäläinen. + 2004-07-28 Anders Carlsson <andersca@gnome.org> * python/dbus.py: diff --git a/bus/activation.c b/bus/activation.c index e286ba9d..b799948f 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -44,7 +44,7 @@ struct BusActivation int refcount; DBusHashTable *entries; DBusHashTable *pending_activations; - char *server_address; + char *server_address; BusContext *context; int n_pending_activations; /**< This is in fact the number of BusPendingActivationEntry, * i.e. number of pending activation requests, not pending @@ -1319,40 +1319,46 @@ bus_activation_activate_service (BusActivation *activation, if (!entry) return FALSE; - /* Check if the service is active */ - _dbus_string_init_const (&service_str, service_name); - if (bus_registry_lookup (bus_context_get_registry (activation->context), &service_str) != NULL) + /* Bypass the registry lookup if we're auto-activating, bus_dispatch would not + * call us if the service is already active. + */ + if (!auto_activation) { - _dbus_verbose ("Service \"%s\" is already active\n", service_name); + /* Check if the service is active */ + _dbus_string_init_const (&service_str, service_name); + if (bus_registry_lookup (bus_context_get_registry (activation->context), &service_str) != NULL) + { + _dbus_verbose ("Service \"%s\" is already active\n", service_name); - message = dbus_message_new_method_return (activation_message); + message = dbus_message_new_method_return (activation_message); - if (!message) - { - _dbus_verbose ("No memory to create reply to activate message\n"); - BUS_SET_OOM (error); - return FALSE; - } + if (!message) + { + _dbus_verbose ("No memory to create reply to activate message\n"); + BUS_SET_OOM (error); + return FALSE; + } - if (!dbus_message_append_args (message, - DBUS_TYPE_UINT32, DBUS_ACTIVATION_REPLY_ALREADY_ACTIVE, - DBUS_TYPE_INVALID)) - { - _dbus_verbose ("No memory to set args of reply to activate message\n"); - BUS_SET_OOM (error); - dbus_message_unref (message); - return FALSE; - } + if (!dbus_message_append_args (message, + DBUS_TYPE_UINT32, DBUS_ACTIVATION_REPLY_ALREADY_ACTIVE, + DBUS_TYPE_INVALID)) + { + _dbus_verbose ("No memory to set args of reply to activate message\n"); + BUS_SET_OOM (error); + dbus_message_unref (message); + return FALSE; + } - retval = bus_transaction_send_from_driver (transaction, connection, message); - dbus_message_unref (message); - if (!retval) - { - _dbus_verbose ("Failed to send reply\n"); - BUS_SET_OOM (error); - } + retval = bus_transaction_send_from_driver (transaction, connection, message); + dbus_message_unref (message); + if (!retval) + { + _dbus_verbose ("Failed to send reply\n"); + BUS_SET_OOM (error); + } - return retval; + return retval; + } } pending_activation_entry = dbus_new0 (BusPendingActivationEntry, 1); @@ -1504,11 +1510,8 @@ bus_activation_activate_service (BusActivation *activation, * argv[0] */ - if (activated == TRUE) - { - pending_activation->babysitter = NULL; - return TRUE; - } + if (activated) + return TRUE; /* Now try to spawn the process */ argv[0] = entry->exec; diff --git a/bus/config-loader-libxml.c b/bus/config-loader-libxml.c index 054acc6e..a1b8f838 100644 --- a/bus/config-loader-libxml.c +++ b/bus/config-loader-libxml.c @@ -30,46 +30,112 @@ #include <errno.h> #include <string.h> -static void* -libxml_malloc (size_t size) -{ - return dbus_malloc (size); -} +/* About the error handling: + * - setup a "structured" error handler that catches structural + * errors and some oom errors + * - assume that a libxml function returning an error code means + * out-of-memory + */ +#define _DBUS_MAYBE_SET_OOM(e) (dbus_error_is_set(e) ? (void)0 : _DBUS_SET_OOM(e)) -static void* -libxml_realloc (void *ptr, size_t size) -{ - return dbus_realloc (ptr, size); -} -static void -libxml_free (void *ptr) +static dbus_bool_t +xml_text_start_element (BusConfigParser *parser, + xmlTextReader *reader, + DBusError *error) { - dbus_free (ptr); + const char *name; + int n_attributes; + const char **attribute_names, **attribute_values; + dbus_bool_t ret; + int i, status, is_empty; + + _DBUS_ASSERT_ERROR_IS_CLEAR (error); + + ret = FALSE; + attribute_names = NULL; + attribute_values = NULL; + + name = xmlTextReaderConstName (reader); + n_attributes = xmlTextReaderAttributeCount (reader); + is_empty = xmlTextReaderIsEmptyElement (reader); + + if (name == NULL || n_attributes < 0 || is_empty == -1) + { + _DBUS_MAYBE_SET_OOM (error); + goto out; + } + + attribute_names = dbus_new0 (const char *, n_attributes + 1); + attribute_values = dbus_new0 (const char *, n_attributes + 1); + if (attribute_names == NULL || attribute_values == NULL) + { + _DBUS_SET_OOM (error); + goto out; + } + i = 0; + while ((status = xmlTextReaderMoveToNextAttribute (reader)) == 1) + { + _dbus_assert (i < n_attributes); + attribute_names[i] = xmlTextReaderConstName (reader); + attribute_values[i] = xmlTextReaderConstValue (reader); + if (attribute_names[i] == NULL || attribute_values[i] == NULL) + { + _DBUS_MAYBE_SET_OOM (error); + goto out; + } + i++; + } + if (status == -1) + { + _DBUS_MAYBE_SET_OOM (error); + goto out; + } + _dbus_assert (i == n_attributes); + + ret = bus_config_parser_start_element (parser, name, + attribute_names, attribute_values, + error); + if (ret && is_empty == 1) + ret = bus_config_parser_end_element (parser, name, error); + + out: + dbus_free (attribute_names); + dbus_free (attribute_values); + + return ret; } -static char* -libxml_strdup (const char *str) +static void xml_shut_up (void *ctx, const char *msg, ...) { - return _dbus_strdup (str); + return; } static void -xml_text_reader_error (void *arg, - const char *msg, - xmlParserSeverities severity, - xmlTextReaderLocatorPtr locator) +xml_text_reader_error (void *arg, xmlErrorPtr xml_error) { DBusError *error = arg; - - if (!dbus_error_is_set (error)) + +#if 0 + _dbus_verbose ("XML_ERROR level=%d, domain=%d, code=%d, msg=%s\n", + xml_error->level, xml_error->domain, + xml_error->code, xml_error->message); +#endif + + if (!dbus_error_is_set (error) && + (xml_error->level == XML_ERR_ERROR || + xml_error->level == XML_ERR_FATAL)) { - dbus_set_error (error, DBUS_ERROR_FAILED, - "Error loading config file: %s", - msg); + if (xml_error->code != XML_ERR_NO_MEMORY) + _DBUS_SET_OOM (error); + else + dbus_set_error (error, DBUS_ERROR_FAILED, + "Error loading config file: '%s'", + xml_error->message); } } + BusConfigParser* bus_config_load (const DBusString *file, dbus_bool_t is_toplevel, @@ -78,66 +144,71 @@ bus_config_load (const DBusString *file, { xmlTextReader *reader; - const char *filename; BusConfigParser *parser; - DBusString dirname; + DBusString dirname, data; + const char *data_str; DBusError tmp_error; int ret; _DBUS_ASSERT_ERROR_IS_CLEAR (error); - filename = _dbus_string_get_const_data (file); parser = NULL; reader = NULL; - dbus_error_init (&tmp_error); - if (xmlMemSetup (libxml_free, - libxml_malloc, - libxml_realloc, - libxml_strdup) != 0) + if (is_toplevel) { - /* Current libxml can't possibly fail here, but just being - * paranoid; don't really know why xmlMemSetup() returns an - * error code, assuming some version of libxml had a reason. - */ - dbus_set_error (error, DBUS_ERROR_FAILED, - "xmlMemSetup() didn't work for some reason\n"); - return NULL; + /* xmlMemSetup only fails if one of the functions is NULL */ + xmlMemSetup (dbus_free, + dbus_malloc, + dbus_realloc, + _dbus_strdup); + xmlInitParser (); + xmlSetGenericErrorFunc (NULL, xml_shut_up); } if (!_dbus_string_init (&dirname)) { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + _DBUS_SET_OOM (error); + return NULL; + } + + if (!_dbus_string_init (&data)) + { + _DBUS_SET_OOM (error); + _dbus_string_free (&dirname); return NULL; } if (!_dbus_string_get_dirname (file, &dirname)) { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + _DBUS_SET_OOM (error); goto failed; } parser = bus_config_parser_new (&dirname, is_toplevel, parent); if (parser == NULL) { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); + _DBUS_SET_OOM (error); goto failed; } - errno = 0; - reader = xmlNewTextReaderFilename (filename); + if (!_dbus_file_get_contents (&data, file, error)) + goto failed; + + data_str = _dbus_string_get_const_data (&data); + reader = xmlReaderForMemory (data_str, _dbus_string_get_length (&data), + NULL, NULL, 0); if (reader == NULL) { - dbus_set_error (error, DBUS_ERROR_FAILED, - "Failed to load configuration file %s: %s\n", - filename, - errno != 0 ? strerror (errno) : "Unknown error"); - + _DBUS_SET_OOM (error); goto failed; } - xmlTextReaderSetErrorHandler (reader, xml_text_reader_error, &tmp_error); + xmlTextReaderSetParserProp (reader, XML_PARSER_SUBST_ENTITIES, 1); + + dbus_error_init (&tmp_error); + xmlTextReaderSetStructuredErrorHandler (reader, xml_text_reader_error, &tmp_error); while ((ret = xmlTextReaderRead (reader)) == 1) { @@ -145,31 +216,87 @@ bus_config_load (const DBusString *file, if (dbus_error_is_set (&tmp_error)) goto reader_out; - - /* "enum" anyone? http://dotgnu.org/pnetlib-doc/System/Xml/XmlNodeType.html for - * the magic numbers - */ + type = xmlTextReaderNodeType (reader); + if (type == -1) + { + _DBUS_MAYBE_SET_OOM (&tmp_error); + goto reader_out; + } + + switch ((xmlReaderTypes) type) { + case XML_READER_TYPE_ELEMENT: + xml_text_start_element (parser, reader, &tmp_error); + break; + + case XML_READER_TYPE_TEXT: + case XML_READER_TYPE_CDATA: + { + DBusString content; + const char *value; + value = xmlTextReaderConstValue (reader); + if (value != NULL) + { + _dbus_string_init_const (&content, value); + bus_config_parser_content (parser, &content, &tmp_error); + } + else + _DBUS_MAYBE_SET_OOM (&tmp_error); + break; + } + + case XML_READER_TYPE_DOCUMENT_TYPE: + { + const char *name; + name = xmlTextReaderConstName (reader); + if (name != NULL) + bus_config_parser_check_doctype (parser, name, &tmp_error); + else + _DBUS_MAYBE_SET_OOM (&tmp_error); + break; + } + + case XML_READER_TYPE_END_ELEMENT: + { + const char *name; + name = xmlTextReaderConstName (reader); + if (name != NULL) + bus_config_parser_end_element (parser, name, &tmp_error); + else + _DBUS_MAYBE_SET_OOM (&tmp_error); + break; + } + + case XML_READER_TYPE_DOCUMENT: + case XML_READER_TYPE_DOCUMENT_FRAGMENT: + case XML_READER_TYPE_PROCESSING_INSTRUCTION: + case XML_READER_TYPE_COMMENT: + case XML_READER_TYPE_ENTITY: + case XML_READER_TYPE_NOTATION: + case XML_READER_TYPE_WHITESPACE: + case XML_READER_TYPE_SIGNIFICANT_WHITESPACE: + case XML_READER_TYPE_END_ENTITY: + case XML_READER_TYPE_XML_DECLARATION: + /* nothing to do, just read on */ + break; + + case XML_READER_TYPE_NONE: + case XML_READER_TYPE_ATTRIBUTE: + case XML_READER_TYPE_ENTITY_REFERENCE: + _dbus_assert_not_reached ("unexpected nodes in XML"); + } + if (dbus_error_is_set (&tmp_error)) goto reader_out; - - /* FIXME I don't really know exactly what I need to do to - * resolve all entities and so on to get the full content of a - * node or attribute value. I'm worried about whether I need to - * manually handle stuff like < - */ } if (ret == -1) - { - if (!dbus_error_is_set (&tmp_error)) - dbus_set_error (&tmp_error, - DBUS_ERROR_FAILED, - "Unknown failure loading configuration file"); - } - + _DBUS_MAYBE_SET_OOM (&tmp_error); + reader_out: xmlFreeTextReader (reader); + if (is_toplevel) + xmlCleanupParser(); reader = NULL; if (dbus_error_is_set (&tmp_error)) { @@ -180,12 +307,14 @@ bus_config_load (const DBusString *file, if (!bus_config_parser_finished (parser, error)) goto failed; _dbus_string_free (&dirname); + _dbus_string_free (&data); _DBUS_ASSERT_ERROR_IS_CLEAR (error); return parser; failed: _DBUS_ASSERT_ERROR_IS_SET (error); _dbus_string_free (&dirname); + _dbus_string_free (&data); if (parser) bus_config_parser_unref (parser); _dbus_assert (reader == NULL); /* must go to reader_out first */ diff --git a/configure.in b/configure.in index 17b34018..2c18dd45 100644 --- a/configure.in +++ b/configure.in @@ -673,10 +673,11 @@ else fi if $dbus_use_libxml ; then - dnl libxml loader is broken - AC_MSG_ERROR([libxml loader is broken ATM, use the expat loader]) + dnl libxml OOM handling is a bit broken + AC_MSG_WARN([libxml loader is not as robust as the expat one wrt. OOM handling]) fi + AM_CONDITIONAL(DBUS_USE_EXPAT, $dbus_use_expat) AM_CONDITIONAL(DBUS_USE_LIBXML, $dbus_use_libxml) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 8887720a..1db8bd9c 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -2622,7 +2622,7 @@ dbus_message_iter_get_args_valist (DBusMessageIter *iter, #endif /* DBUS_HAVE_INT64 */ case DBUS_TYPE_DOUBLE: { - void *ptr = va_arg (var_args, void *); + void *ptr = va_arg (var_args, double *); _dbus_message_iter_get_basic_type (iter, spec_type, ptr); break; } @@ -6696,6 +6696,7 @@ verify_test_message (DBusMessage *message) char *our_str; double our_double; dbus_bool_t our_bool; + unsigned char our_byte_1, our_byte_2; dbus_uint32_t our_uint32; dbus_int32_t *our_uint32_array; int our_uint32_array_len; @@ -6730,6 +6731,9 @@ verify_test_message (DBusMessage *message) DBUS_TYPE_STRING, &our_str, DBUS_TYPE_DOUBLE, &our_double, DBUS_TYPE_BOOLEAN, &our_bool, + DBUS_TYPE_BYTE, &our_byte_1, + DBUS_TYPE_BYTE, &our_byte_2, + DBUS_TYPE_NIL, DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32, &our_uint32_array, &our_uint32_array_len, DBUS_TYPE_ARRAY, DBUS_TYPE_INT32, @@ -6775,6 +6779,12 @@ verify_test_message (DBusMessage *message) if (!our_bool) _dbus_assert_not_reached ("booleans differ"); + if (our_byte_1 != 42) + _dbus_assert_not_reached ("bytes differ!"); + + if (our_byte_2 != 24) + _dbus_assert_not_reached ("bytes differ!"); + if (our_uint32_array_len != 4 || our_uint32_array[0] != 0x12345678 || our_uint32_array[1] != 0x23456781 || @@ -7027,6 +7037,9 @@ _dbus_message_test (const char *test_data_dir) DBUS_TYPE_STRING, "Test string", DBUS_TYPE_DOUBLE, 3.14159, DBUS_TYPE_BOOLEAN, TRUE, + DBUS_TYPE_BYTE, (unsigned char) 42, + DBUS_TYPE_BYTE, 24, + DBUS_TYPE_NIL, DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32, our_uint32_array, _DBUS_N_ELEMENTS (our_uint32_array), DBUS_TYPE_ARRAY, DBUS_TYPE_INT32, our_int32_array, @@ -7062,6 +7075,9 @@ _dbus_message_test (const char *test_data_dir) sig[i++] = DBUS_TYPE_STRING; sig[i++] = DBUS_TYPE_DOUBLE; sig[i++] = DBUS_TYPE_BOOLEAN; + sig[i++] = DBUS_TYPE_BYTE; + sig[i++] = DBUS_TYPE_BYTE; + sig[i++] = DBUS_TYPE_NIL; sig[i++] = DBUS_TYPE_ARRAY; sig[i++] = DBUS_TYPE_UINT32; sig[i++] = DBUS_TYPE_ARRAY; diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c index 2e942abc..37825f1c 100644 --- a/dbus/dbus-transport-unix.c +++ b/dbus/dbus-transport-unix.c @@ -717,11 +717,25 @@ unix_handle_watch (DBusTransport *transport, _dbus_assert (watch == unix_transport->read_watch || watch == unix_transport->write_watch); + /* Disconnect in case of an error. In case of hangup do not + * disconnect the transport because data can still be in the buffer + * and do_reading may need several iteration to read it all (because + * of its max_bytes_read_per_iteration limit). The condition where + * flags == HANGUP (without READABLE) probably never happen in fact. + */ + if ((flags & DBUS_WATCH_ERROR) || + ((flags & DBUS_WATCH_HANGUP) && !(flags & DBUS_WATCH_READABLE))) + { + _dbus_verbose ("Hang up or error on watch\n"); + _dbus_transport_disconnect (transport); + return TRUE; + } + if (watch == unix_transport->read_watch && (flags & DBUS_WATCH_READABLE)) { -#if 1 - _dbus_verbose ("handling read watch\n"); +#if 0 + _dbus_verbose ("handling read watch (%x)\n", flags); #endif if (!do_authentication (transport, TRUE, FALSE)) return FALSE; @@ -763,12 +777,6 @@ unix_handle_watch (DBusTransport *transport, } #endif /* DBUS_ENABLE_VERBOSE_MODE */ - if (flags & (DBUS_WATCH_HANGUP | DBUS_WATCH_ERROR)) - { - _dbus_transport_disconnect (transport); - return TRUE; - } - return TRUE; } diff --git a/glib/Makefile.am b/glib/Makefile.am index d6f00d07..e0362b42 100644 --- a/glib/Makefile.am +++ b/glib/Makefile.am @@ -39,7 +39,7 @@ bin_PROGRAMS=dbus-glib-tool dbus_glib_tool_SOURCES = \ dbus-glib-tool.c -dbus_glib_tool_LDADD= $(DBUS_GLIB_TOOL_LIBS) libdbus-gtool.la +dbus_glib_tool_LDADD= -lexpat libdbus-gtool.la if DBUS_BUILD_TESTS diff --git a/test/data/invalid-config-files/not-well-formed.conf b/test/data/invalid-config-files/not-well-formed.conf new file mode 100644 index 00000000..9e59cbc0 --- /dev/null +++ b/test/data/invalid-config-files/not-well-formed.conf @@ -0,0 +1,5 @@ +<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" + "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd"> +<busconfig> + <user>mybususer</foo> +</busconfig> diff --git a/test/data/invalid-config-files/truncated-file.conf b/test/data/invalid-config-files/truncated-file.conf new file mode 100644 index 00000000..e8d60883 --- /dev/null +++ b/test/data/invalid-config-files/truncated-file.conf @@ -0,0 +1,9 @@ +<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" + "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd"> +<busconfig> + <user>mybususer</user> + <listen>unix:path=/foo/bar</listen> + <listen>tcp:port=1234</listen> + <includedir>basic.d</includedir> + <servicedir>/usr/share/foo</servicedir> + <include ignore_missing="y |