[systemd-commits] 3 commits - src/libsystemd test/udev-test.pl

[Tom Gundersen]

 src/libsystemd/sd-device/sd-device.c |   41 +++++++++++++++++------------------
 test/udev-test.pl                    |    2 -
 2 files changed, 22 insertions(+), 21 deletions(-)

New commits:
commit 2fe29a46a3e16a787098f9fb410e1b756506fd52
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Apr 3 17:43:13 2015 +0200

    sd-device: get_sysattr_value - don't leak returned value
    
    This was getting leaked as a copy was added to the hashmap, simply add the
    returned value instead.
    
    This should fix CID #1292806.

diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
index 3179d16..0dd0e14 100644
--- a/src/libsystemd/sd-device/sd-device.c
+++ b/src/libsystemd/sd-device/sd-device.c
@@ -1593,9 +1593,9 @@ _public_ int sd_device_get_property_value(sd_device *device, const char *key, co
 }
 
 /* replaces the value if it already exists */
-static int device_add_sysattr_value(sd_device *device, const char *_key, const char *_value) {
+static int device_add_sysattr_value(sd_device *device, const char *_key, char *value) {
         _cleanup_free_ char *key = NULL;
-        _cleanup_free_ char *value = NULL;
+        _cleanup_free_ char *value_old = NULL;
         int r;
 
         assert(device);
@@ -1605,28 +1605,18 @@ static int device_add_sysattr_value(sd_device *device, const char *_key, const c
         if (r < 0)
                 return r;
 
-        value = hashmap_remove2(device->sysattr_values, _key, (void **)&key);
+        value_old = hashmap_remove2(device->sysattr_values, _key, (void **)&key);
         if (!key) {
                 key = strdup(_key);
                 if (!key)
                         return -ENOMEM;
         }
 
-        free(value);
-        value = NULL;
-
-        if (_value) {
-                value = strdup(_value);
-                if (!value)
-                        return -ENOMEM;
-        }
-
         r = hashmap_put(device->sysattr_values, key, value);
         if (r < 0)
                 return r;
 
         key = NULL;
-        value = NULL;
 
         return 0;
 }
@@ -1740,8 +1730,9 @@ static void device_remove_sysattr_value(sd_device *device, const char *_key) {
 
 /* set the attribute and save it in the cache. If a NULL value is passed the
  * attribute is cleared from the cache */
-_public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr, char *value) {
+_public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr, char *_value) {
         _cleanup_close_ int fd = -1;
+        _cleanup_free_ char *value = NULL;
         const char *syspath;
         char *path;
         struct stat statbuf;
@@ -1752,7 +1743,7 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
         assert_return(device, -EINVAL);
         assert_return(sysattr, -EINVAL);
 
-        if (!value) {
+        if (!_value) {
                 device_remove_sysattr_value(device, sysattr);
 
                 return 0;
@@ -1765,7 +1756,11 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
         path = strjoina(syspath, "/", sysattr);
         r = lstat(path, &statbuf);
         if (r < 0) {
-                r = device_add_sysattr_value(device, sysattr, "");
+                value = strdup("");
+                if (!value)
+                        return -ENOMEM;
+
+                r = device_add_sysattr_value(device, sysattr, value);
                 if (r < 0)
                         return r;
 
@@ -1783,11 +1778,11 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
         if ((statbuf.st_mode & S_IRUSR) == 0)
                 return -EACCES;
 
-        value_len = strlen(value);
+        value_len = strlen(_value);
 
         /* drop trailing newlines */
-        while (value_len > 0 && value[--value_len] == '\n')
-                value[value_len] = '\0';
+        while (value_len > 0 && _value[--value_len] == '\n')
+                _value[value_len] = '\0';
 
         /* value length is limited to 4k */
         if (value_len > 4096)
@@ -1797,6 +1792,10 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
         if (fd < 0)
                 return -errno;
 
+        value = strdup(_value);
+        if (!value)
+                return -ENOMEM;
+
         size = write(fd, value, value_len);
         if (size < 0)
                 return -errno;
@@ -1808,5 +1807,7 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
         if (r < 0)
                 return r;
 
+        value = NULL;
+
         return 0;
 }

commit b3f2490072eb0495d4941dba390a7f759d28c663
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Apr 3 17:27:45 2015 +0200

    test: udev, valgrind - track origins of uninitiaziled memory

diff --git a/test/udev-test.pl b/test/udev-test.pl
index 126eab9..64d7f93 100755
--- a/test/udev-test.pl
+++ b/test/udev-test.pl
@@ -23,7 +23,7 @@ use strict;
 my $udev_bin            = "./test-udev";
 my $valgrind            = 0;
 my $gdb                 = 0;
-my $udev_bin_valgrind   = "valgrind --tool=memcheck --leak-check=yes --quiet $udev_bin";
+my $udev_bin_valgrind   = "valgrind --tool=memcheck --leak-check=yes --track-origins=yes --quiet $udev_bin";
 my $udev_bin_gdb        = "gdb --args $udev_bin";
 my $udev_dev            = "test/dev";
 my $udev_run            = "test/run";

commit afcac065c0f649ebcf0f450475a8d7c3bc776d14
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Apr 3 17:27:00 2015 +0200

    sd-device: initialize variable
    
    sysnum would not be initialized if sysname had no trailing digits.

diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
index da1052c..3179d16 100644
--- a/src/libsystemd/sd-device/sd-device.c
+++ b/src/libsystemd/sd-device/sd-device.c
@@ -930,7 +930,7 @@ _public_ int sd_device_get_devname(sd_device *device, const char **devname) {
 
 static int device_set_sysname(sd_device *device) {
         _cleanup_free_ char *sysname = NULL;
-        const char *sysnum;
+        const char *sysnum = NULL;
         const char *pos;
         size_t len = 0;