[haiku-commits] haiku: hrev48676 - src/system/libroot/os

  • From: pulkomandy@xxxxxxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Wed, 14 Jan 2015 11:53:25 +0100 (CET)

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
 


Other related posts: