build: avoid unsafe functions in libgen.h

POSIX says that both basename() and dirname() may return static
storage (aka they need not be thread-safe); and that they may but
not must modify their input argument.  Furthermore, <libgen.h>
is not available on all platforms.  For these reasons, you should
never use these functions in a multi-threaded library.

Gnulib instead recommends a way to avoid the portability nightmare:
gnulib's "dirname.h" provides useful thread-safe counterparts.  The
obvious dir_name() and base_name() are GPL (because they malloc(),
but call exit() on failure) so we can't use them; but the LGPL
variants mdir_name() (malloc's or returns NULL) and last_component
(always points into the incoming string without modifying it,
differing from basename semantics only on corner cases like the
empty string that we shouldn't be hitting in the first place) are
already in use in libvirt.  This finishes the swap over to the safe
functions.

* cfg.mk (sc_prohibit_libgen): New rule.
* src/util/vircgroup.c: Fix offenders.
* src/parallels/parallels_storage.c (parallelsPoolAddByDomain):
Likewise.
* src/parallels/parallels_network.c (parallelsGetBridgedNetInfo):
Likewise.
* src/node_device/node_device_udev.c (udevProcessSCSIHost)
(udevProcessSCSIDevice): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskDeleteVol): Likewise.
* src/util/virpci.c (virPCIGetDeviceAddressFromSysfsLink):
Likewise.
* src/util/virstoragefile.h (_virStorageFileMetadata): Avoid false
positive.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/cfg.mk b/cfg.mk
index ebb6613d9f638d22670cd677ca942c0ee567ad0b..f0f78f5f2d002141062a5f43cad063d049187857 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -493,6 +493,12 @@ sc_prohibit_gethostby:
        halt='use getaddrinfo, not gethostby*'                          \
          $(_sc_search_regexp)
 
+# dirname and basename from <libgen.h> are not required to be thread-safe
+sc_prohibit_libgen:
+       @prohibit='( (base|dir)name *\(|include .libgen\.h)'            \
+       halt='use functions from gnulib "dirname.h", not <libgen.h>'    \
+         $(_sc_search_regexp)
+
 # raw xmlGetProp requires some nasty casts
 sc_prohibit_xmlGetProp:
        @prohibit='\<xmlGetProp *\('                                    \

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 92292be9407dcf752d0e57c6333e5f5be4ce9e7d..f98841e96f1b6991d22e6ccaee061a3d52c458e1 100644 (file)
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1,7 +1,7 @@
 /*
  * node_device_udev.c: node device enumeration - libudev implementation
  *
- * Copyright (C) 2009-2012 Red Hat, Inc.
+ * Copyright (C) 2009-2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -26,6 +26,7 @@
 #include <scsi/scsi.h>
 #include <c-ctype.h>
 
+#include "dirname.h"
 #include "node_device_udev.h"
 #include "virerror.h"
 #include "node_device_conf.h"
@@ -653,7 +654,7 @@ static int udevProcessSCSIHost(struct udev_device *device ATTRIBUTE_UNUSED,
     union _virNodeDevCapData *data = &def->caps->data;
     char *filename = NULL;
 
-    filename = basename(def->sysfs_path);
+    filename = last_component(def->sysfs_path);
 
     if (!STRPREFIX(filename, "host")) {
         VIR_ERROR(_("SCSI host found, but its udev name '%s' does "
@@ -774,7 +775,7 @@ static int udevProcessSCSIDevice(struct udev_device *device ATTRIBUTE_UNUSED,
     union _virNodeDevCapData *data = &def->caps->data;
     char *filename = NULL, *p = NULL;
 
-    filename = basename(def->sysfs_path);
+    filename = last_component(def->sysfs_path);
 
     if (udevStrToLong_ui(filename, &p, 10, &data->scsi.host) == -1) {
         goto out;

diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index c61e2802254f628c7b92f1749e93e78f803a5b96..7a9436f8608205044b6c37a49b7211920e5812c7 100644 (file)
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -2,6 +2,7 @@
  * parallels_storage.c: core privconn functions for managing
  * Parallels Cloud Server hosts
  *
+ * Copyright (C) 2013 Red Hat, Inc.
  * Copyright (C) 2012 Parallels, Inc.
  *
  * This library is free software; you can redistribute it and/or
@@ -23,6 +24,7 @@
 #include <config.h>
 
 #include "datatypes.h"
+#include "dirname.h"
 #include "viralloc.h"
 #include "virerror.h"
 #include "md5.h"
@@ -64,7 +66,7 @@ static int parallelsGetBridgedNetInfo(virNetworkDefPtr def, virJSONValuePtr jobj
         goto cleanup;
     }
 
-    if (!(def->bridge = strdup(basename(bridgePath)))) {
+    if (!(def->bridge = strdup(last_component(bridgePath)))) {
         virReportOOMError();
         goto cleanup;
     }

diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c
index cf2f790a421a65aa15d42b80917228595057be28..271f3709dc17aeb5b54008840dc8f150eb4b78e5 100644 (file)
--- a/src/parallels/parallels_storage.c
+++ b/src/parallels/parallels_storage.c
@@ -2,6 +2,7 @@
  * parallels_storage.c: core driver functions for managing
  * Parallels Cloud Server hosts
  *
+ * Copyright (C) 2013 Red Hat, Inc.
  * Copyright (C) 2012 Parallels, Inc.
  *
  * This library is free software; you can redistribute it and/or
@@ -28,9 +29,9 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
-#include <libgen.h>
 
 #include "datatypes.h"
+#include "dirname.h"
 #include "viralloc.h"
 #include "configmake.h"
 #include "virstoragefile.h"
@@ -230,13 +231,12 @@ parallelsPoolAddByDomain(virConnectPtr conn, virDomainObjPtr dom)
     virStoragePoolObjPtr pool = NULL;
     int j;
 
-    if (!(poolPath = strdup(pdom->home))) {
+    poolPath = mdir_name(pdom->home);
+    if (!poolPath) {
         virReportOOMError();
         return NULL;
     }
 
-    poolPath = dirname(poolPath);
-
     for (j = 0; j < pools->count; j++) {
         if (STREQ(poolPath, pools->objs[j]->def->target.path)) {
             pool = pools->objs[j];

diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 40da3066124bd01e10b53dc1ee08a2ce79f5b22a..1faf1aed36c52d24a70ea0795545ecaa6a5a4cf8 100644 (file)
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -26,6 +26,7 @@
 #include <unistd.h>
 #include <stdio.h>
 
+#include "dirname.h"
 #include "virerror.h"
 #include "virlog.h"
 #include "storage_backend_disk.h"
@@ -728,8 +729,8 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    dev_name = basename(devpath);
-    srcname = basename(pool->def->source.devices[0].path);
+    dev_name = last_component(devpath);
+    srcname = last_component(pool->def->source.devices[0].path);
     VIR_DEBUG("dev_name=%s, srcname=%s", dev_name, srcname);
 
     isDevMapperDevice = virIsDevMapperDevice(devpath);

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 5a2a75c9a3a0c832614e2eff93652e2ca6499f8e..4c836c756996139d72067260f331bc4238718969 100644 (file)
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1,7 +1,7 @@
 /*
  * vircgroup.c: methods for managing control cgroups
  *
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 Red Hat, Inc.
  * Copyright IBM Corp. 2008
  *
  * This library is free software; you can redistribute it and/or
@@ -37,7 +37,6 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <signal.h>
-#include <libgen.h>
 #include <dirent.h>
 
 #define __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__

diff --git a/src/util/virpci.c b/src/util/virpci.c
index e58010b42e34abc51dd9a9b3897eef52e1c7f4f9..5811f22c0518e674cb58890186ee9c715194fb0b 100644 (file)
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -36,6 +36,7 @@
 #include <unistd.h>
 #include <stdlib.h>
 
+#include "dirname.h"
 #include "virlog.h"
 #include "viralloc.h"
 #include "vircommand.h"
@@ -1925,7 +1926,7 @@ virPCIGetDeviceAddressFromSysfsLink(const char *device_link,
         return ret;
     }
 
-    config_address = basename(device_path);
+    config_address = last_component(device_path);
     if (VIR_ALLOC(*bdf) != 0) {
         virReportOOMError();
         goto out;

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index d7b4508467ed9b3b493c3765db43932f7107de69..ffe7a54039edc803006525c3ab4bb52308210b56 100644 (file)
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -56,7 +56,7 @@ typedef virStorageFileMetadata *virStorageFileMetadataPtr;
 struct _virStorageFileMetadata {
     char *backingStore; /* Canonical name (absolute file, or protocol) */
     char *backingStoreRaw; /* If file, original name, possibly relative */
-    char *directory; /* The directory containing basename(backingStoreRaw) */
+    char *directory; /* The directory containing basename of backingStoreRaw */
     int backingStoreFormat; /* enum virStorageFileFormat */
     bool backingStoreIsFile;
     virStorageFileMetadataPtr backingMeta;