hrev48676 adds 2 changesets to branch 'master' old head: 98731302d80db5603b3483ea6697a775cd89d86d new head: bcb793d37b1e20d0329ea5bafa5095198fa8f7d2 overview: http://cgit.haiku-os.org/haiku/log/?qt=range&q=bcb793d37b1e+%5E98731302d80d ---------------------------------------------------------------------------- 0687a01b53e4: driver_settings: don't strdup(NULL) * This is not allowed by strdup POSIX specs and GCC may use its builtin strdup which doesn't check for it. * also refactor parse_driver_settings_string to create the settings_handle using settings_new, to reduce code duplication. bcb793d37b1e: Fix driver_settings in kernel mode outside of drivers. The API allows to create driver settings which are not added to the global list, however those were left partially uninitialized, and there was no way to cleanly delete them. Tag such unattached settings with a ref_count of -1, and have delete_driver_settings check for this and handle the case correctly. Note: #10494 comment 2 says the settings for packagefs shouldn't be added to the kernel driver settings list, which is why I went with this solution. An alternative would be always using the list and the reference counting, but I don't know what the consequences are. Fixes #10494. [ Adrien Destugues <pulkomandy@xxxxxxxxx> ] ---------------------------------------------------------------------------- 1 file changed, 26 insertions(+), 23 deletions(-) src/system/libroot/os/driver_settings.cpp | 49 ++++++++++++++------------- ############################################################################ Commit: 0687a01b53e495083e03aaa50bb223269575b11c URL: http://cgit.haiku-os.org/haiku/commit/?id=0687a01b53e4 Author: Adrien Destugues <pulkomandy@xxxxxxxxx> Date: Wed Jan 14 09:54:59 2015 UTC driver_settings: don't strdup(NULL) * This is not allowed by strdup POSIX specs and GCC may use its builtin strdup which doesn't check for it. * also refactor parse_driver_settings_string to create the settings_handle using settings_new, to reduce code duplication. ---------------------------------------------------------------------------- diff --git a/src/system/libroot/os/driver_settings.cpp b/src/system/libroot/os/driver_settings.cpp index 029e850..9a986ae 100644 --- a/src/system/libroot/os/driver_settings.cpp +++ b/src/system/libroot/os/driver_settings.cpp @@ -820,21 +820,16 @@ load_driver_settings_file(int fd) void * parse_driver_settings_string(const char *settingsString) { + if (settingsString == NULL) + return NULL; + // we simply copy the whole string to use it as our internal buffer char *text = strdup(settingsString); - if (settingsString == NULL || text != NULL) { - settings_handle *handle - = (settings_handle*)malloc(sizeof(settings_handle)); - if (handle != NULL) { - handle->magic = SETTINGS_MAGIC; - handle->text = text; - - if (parse_settings(handle) == B_OK) - return handle; - - free(handle); - } - free(text); + if (text != NULL) { + settings_handle *handle = new_settings(text, NULL); + if (handle == NULL) + free(text); + return handle; } return NULL; ############################################################################ Revision: hrev48676 Commit: bcb793d37b1e20d0329ea5bafa5095198fa8f7d2 URL: http://cgit.haiku-os.org/haiku/commit/?id=bcb793d37b1e Author: Adrien Destugues <pulkomandy@xxxxxxxxx> Date: Wed Jan 14 10:47:51 2015 UTC Ticket: https://dev.haiku-os.org/ticket/10494 Fix driver_settings in kernel mode outside of drivers. The API allows to create driver settings which are not added to the global list, however those were left partially uninitialized, and there was no way to cleanly delete them. Tag such unattached settings with a ref_count of -1, and have delete_driver_settings check for this and handle the case correctly. Note: #10494 comment 2 says the settings for packagefs shouldn't be added to the kernel driver settings list, which is why I went with this solution. An alternative would be always using the list and the reference counting, but I don't know what the consequences are. Fixes #10494. ---------------------------------------------------------------------------- diff --git a/src/system/libroot/os/driver_settings.cpp b/src/system/libroot/os/driver_settings.cpp index 9a986ae..065092e 100644 --- a/src/system/libroot/os/driver_settings.cpp +++ b/src/system/libroot/os/driver_settings.cpp @@ -77,6 +77,8 @@ typedef struct settings_handle { list_link link; char name[B_OS_NAME_LENGTH]; int32 ref_count; + // A negative ref_count means the node is not reference counted and not + // stored in the list. #endif int32 magic; struct driver_settings settings; @@ -101,8 +103,9 @@ static mutex sLock = MUTEX_INITIALIZER("driver settings"); /*! - Returns true for any characters that separate parameters - - those are ignored in the input stream and won't be added + \brief Returns true for any characters that separate parameters + + Those characters are ignored in the input stream and won't be added to any words. */ static inline bool @@ -112,9 +115,8 @@ is_parameter_separator(char c) } -/** Indicates if "c" begins a new word or not. - */ - +/*! Indicates if "c" begins a new word or not. +*/ static inline bool is_word_break(char c) { @@ -413,6 +415,9 @@ new_settings(char *buffer, const char *driverName) if (driverName != NULL) { handle->ref_count = 1; strlcpy(handle->name, driverName, sizeof(handle->name)); + } else { + handle->ref_count = -1; + handle->name[0] = 0; } #endif @@ -687,11 +692,14 @@ unload_driver_settings(void *_handle) #ifdef _KERNEL_MODE mutex_lock(&sLock); - if (--handle->ref_count == 0 && gBootDevice > 0) { - // don't unload an handle when /boot is not available - list_remove_link(&handle->link); - } else - handle = NULL; + + if (handle->ref_count > 0) { + if (--handle->ref_count == 0 && gBootDevice > 0) { + // don't unload an handle when /boot is not available + list_remove_link(&handle->link); + } else + handle = NULL; + } mutex_unlock(&sLock); #endif