maint: prohibit access(,X_OK)

This simplifies several callers that were repeating checks already
guaranteed by util.c, and makes other callers more robust to now
reject directories.  remote_driver.c was over-strict - access(,R_OK)
is only needed to execute a script file; a binary only needs
access(,X_OK) (besides, it's unusual to see a file with x but not
r permissions, whether script or binary).

* cfg.mk (sc_prohibit_access_xok): New syntax-check rule.
(exclude_file_name_regexp--sc_prohibit_access_xok): Exempt one use.
* src/network/bridge_driver.c (networkStartRadvd): Fix offenders.
* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes)
(qemuCapsInitGuest, qemuCapsInit, qemuCapsExtractVersionInfo):
Likewise.
* src/remote/remote_driver.c (remoteFindDaemonPath): Likewise.
* src/uml/uml_driver.c (umlStartVMDaemon): Likewise.
* src/util/hooks.c (virHookCheck): Likewise.

cfg.mk

@@ -258,6 +258,13 @@ sc_prohibit_fork_wrappers:
 	halt='use virCommand for child processes'			\
 	  $(_sc_search_regexp)
 
+# access with X_OK accepts directories, but we can't exec() those.
+# access with F_OK or R_OK is okay, though.
+sc_prohibit_access_xok:
+	@prohibit='access''(at)? *\(.*X_OK'				\
+	halt='use virFileIsExecutable instead of access''(,X_OK)'	\
+	  $(_sc_search_regexp)
+
 # Similar to the gnulib maint.mk rule for sc_prohibit_strcmp
 # Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0.
 sc_prohibit_strncmp:
@@ -551,6 +558,8 @@ exclude_file_name_regexp--sc_po_check = ^docs/
 exclude_file_name_regexp--sc_prohibit_VIR_ERR_NO_MEMORY = \
   ^(include/libvirt/virterror\.h|daemon/dispatch\.c|src/util/virterror\.c)$$
 
+exclude_file_name_regexp--sc_prohibit_access_xok = ^src/util/util\.c$$
+
 exclude_file_name_regexp--sc_prohibit_always_true_header_tests = \
   (^docs|^python/(libvirt-override|typewrappers)\.c$$)
 

src/network/bridge_driver.c

@@ -689,7 +689,7 @@ networkStartRadvd(virNetworkObjPtr network)
 
     network->radvdPid = -1;
 
-    if (access(RADVD, X_OK) < 0) {
+    if (!virFileIsExecutable(RADVD)) {
         virReportSystemError(errno,
                              _("Cannot find %s - "
                                "Possibly the package isn't installed"),

src/qemu/qemu_capabilities.c

@@ -178,7 +178,7 @@ qemuCapsProbeMachineTypes(const char *binary,
      * Technically we could catch the exec() failure, but that's
      * in a sub-process so it's hard to feed back a useful error.
      */
-    if (access(binary, X_OK) < 0) {
+    if (!virFileIsExecutable(binary)) {
         virReportSystemError(errno, _("Cannot find QEMU binary %s"), binary);
         return -1;
     }
@@ -451,12 +451,9 @@ qemuCapsInitGuest(virCapsPtr caps,
      */
     binary = virFindFileInPath(info->binary);
 
-    if (binary == NULL || access(binary, X_OK) != 0) {
+    if (binary == NULL || !virFileIsExecutable(binary)) {
         VIR_FREE(binary);
         binary = virFindFileInPath(info->altbinary);
-
-        if (binary != NULL && access(binary, X_OK) != 0)
-            VIR_FREE(binary);
     }
 
     /* Can use acceleration for KVM/KQEMU if
@@ -475,10 +472,8 @@ qemuCapsInitGuest(virCapsPtr caps,
             for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
                 kvmbin = virFindFileInPath(kvmbins[i]);
 
-                if (kvmbin == NULL || access(kvmbin, X_OK) != 0) {
-                    VIR_FREE(kvmbin);
+                if (!kvmbin)
                     continue;
-                }
 
                 haskvm = 1;
                 if (!binary)
@@ -753,7 +748,7 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps)
     /* Then possibly the Xen paravirt guests (ie Xenner */
     xenner = virFindFileInPath("xenner");
 
-    if (xenner != NULL && access(xenner, X_OK) == 0 &&
+    if (xenner != NULL && virFileIsExecutable(xenner) == 0 &&
         access("/dev/kvm", F_OK) == 0) {
         for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_xen) ; i++)
             /* Allow Xen 32-on-32, 32-on-64 and 64-on-64 */
@@ -1147,7 +1142,7 @@ int qemuCapsExtractVersionInfo(const char *qemu, const char *arch,
      * Technically we could catch the exec() failure, but that's
      * in a sub-process so it's hard to feed back a useful error.
      */
-    if (access(qemu, X_OK) < 0) {
+    if (!virFileIsExecutable(qemu)) {
         virReportSystemError(errno, _("Cannot find QEMU binary %s"), qemu);
         return -1;
     }

src/remote/remote_driver.c

@@ -306,7 +306,7 @@ remoteFindDaemonPath(void)
         return(customDaemon);
 
     for (i = 0; serverPaths[i]; i++) {
-        if (access(serverPaths[i], X_OK | R_OK) == 0) {
+        if (virFileIsExecutable(serverPaths[i])) {
             return serverPaths[i];
         }
     }

src/uml/uml_driver.c

@@ -829,7 +829,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
      * Technically we could catch the exec() failure, but that's
      * in a sub-process so its hard to feed back a useful error
      */
-    if (access(vm->def->os.kernel, X_OK) < 0) {
+    if (!virFileIsExecutable(vm->def->os.kernel)) {
         virReportSystemError(errno,
                              _("Cannot find UML kernel %s"),
                              vm->def->os.kernel);

src/util/hooks.c

@@ -1,7 +1,7 @@
 /*
  * hooks.c: implementation of the synchronous hooks support
  *
- * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2010-2011 Red Hat, Inc.
  * Copyright (C) 2010 Daniel Veillard
  *
  * This library is free software; you can redistribute it and/or
@@ -94,13 +94,12 @@ static int virHooksFound = -1;
 static int
 virHookCheck(int no, const char *driver) {
     char *path;
-    struct stat sb;
     int ret;
 
     if (driver == NULL) {
         virHookReportError(VIR_ERR_INTERNAL_ERROR,
                    _("Invalid hook name for #%d"), no);
-        return(-1);
+        return -1;
     }
 
     ret = virBuildPath(&path, LIBVIRT_HOOK_DIR, driver);
@@ -108,24 +107,19 @@ virHookCheck(int no, const char *driver) {
         virHookReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Failed to build path for %s hook"),
                            driver);
-        return(-1);
+        return -1;
     }
 
-    if (stat(path, &sb) < 0) {
+    if (!virFileIsExecutable(path)) {
         ret = 0;
-        VIR_DEBUG("No hook script %s", path);
+        VIR_WARN("Missing or non-executable hook script %s", path);
     } else {
-        if ((access(path, X_OK) != 0) || (!S_ISREG(sb.st_mode))) {
-            ret = 0;
-            VIR_WARN("Non executable hook script %s", path);
-        } else {
-            ret = 1;
-            VIR_DEBUG("Found hook script %s", path);
-        }
+        ret = 1;
+        VIR_DEBUG("Found hook script %s", path);
     }
 
     VIR_FREE(path);
-    return(ret);
+    return ret;
 }
 
 /*