From 030ce43b4952a4ff37486f119caaea43f88f3699 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Thu, 27 Jan 2011 15:16:14 -0700
Subject: [PATCH] maint: reject raw close, popen in 'make syntax-check'

commit f1fe9671e was supposed to make sure we use files.h
macros to avoid double close, but it didn't work.

Meanwhile, virCommand is vastly superior to system(), fork(),
and popen() (also to virExec, but we haven't completed that
conversion), so enforce that, too.

* cfg.mk (sc_prohibit_close): Fix typo that excluded close, and
add pclose.
(sc_prohibit_fork_wrappers): New rule, for fork, system, and popen.
* .x-sc_prohibit_close: More exemptions.
* .x-sc_prohibit_fork_wrappers: New file.
* Makefile.am (syntax_check_exceptions): Ship new file.
* src/datatypes.c (virReleaseConnect): Tweak comment to avoid
false positive.
* src/util/files.h (VIR_CLOSE): Likewise.
---
 .x-sc_prohibit_close         | 8 +++++++-
 .x-sc_prohibit_fork_wrappers | 8 ++++++++
 Makefile.am                  | 1 +
 cfg.mk                       | 9 ++++++++-
 src/datatypes.c              | 2 +-
 src/util/files.h             | 4 ++--
 6 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 .x-sc_prohibit_fork_wrappers

diff --git a/.x-sc_prohibit_close b/.x-sc_prohibit_close
index 348200c337..ab14617bd7 100644
--- a/.x-sc_prohibit_close
+++ b/.x-sc_prohibit_close
@@ -1,3 +1,9 @@
+# Non-C files:
 ^docs/.*
+^ChangeLog*
 ^HACKING$
-^src/util/files.c$
+*\.py$
+# Wrapper implementation:
+^src/util/files\.c$
+# Only uses close in documentation comments:
+^src/libvirt\.c$
diff --git a/.x-sc_prohibit_fork_wrappers b/.x-sc_prohibit_fork_wrappers
new file mode 100644
index 0000000000..7f8fc6cd46
--- /dev/null
+++ b/.x-sc_prohibit_fork_wrappers
@@ -0,0 +1,8 @@
+^docs/.*
+^HACKING$
+^src/util/util\.c$
+^tests/testutils\.c$
+# Files that we may want to convert over to virCommand someday...
+^daemon/libvirtd\.c$
+^src/libvirt\.c$
+^src/lxc/lxc_controller\.c$
diff --git a/Makefile.am b/Makefile.am
index 36463f5470..597ec6112c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -25,6 +25,7 @@ syntax_check_exceptions = \
   .x-sc_prohibit_asprintf \
   .x-sc_prohibit_close \
   .x-sc_prohibit_empty_lines_at_EOF \
+  .x-sc_prohibit_fork_wrappers \
   .x-sc_prohibit_gethostby \
   .x-sc_prohibit_gethostname \
   .x-sc_prohibit_gettext_noop \
diff --git a/cfg.mk b/cfg.mk
index db6863db7e..2cc6f90d8d 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -241,13 +241,20 @@ sc_avoid_write:
 
 # Avoid functions that can lead to double-close bugs.
 sc_prohibit_close:
-	@prohibit='\<[f]close *\('					\
+	@prohibit='([^>.]|^)\<[fp]?close *\('				\
 	halt='use VIR_{FORCE_}[F]CLOSE instead of [f]close'		\
 	  $(_sc_search_regexp)
 	@prohibit='\<fdopen *\('					\
 	halt='use VIR_FDOPEN instead of fdopen'				\
 	  $(_sc_search_regexp)
 
+# Prefer virCommand for all child processes.
+# XXX - eventually, we want to enhance this to also prohibit virExec.
+sc_prohibit_fork_wrappers:
+	@prohibit='= *\<(fork|popen|system) *\('			\
+	halt='use virCommand for child processes'			\
+	  $(_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:
diff --git a/src/datatypes.c b/src/datatypes.c
index 7f2d0c5400..e26e332053 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -246,7 +246,7 @@ virReleaseConnect(virConnectPtr conn) {
     DEBUG("release connection %p", conn);
 
     /* make sure to release the connection lock before we call the
-     * close() callbacks, otherwise we will deadlock if an error
+     * close callbacks, otherwise we will deadlock if an error
      * is raised by any of the callbacks */
     virMutexUnlock(&conn->lock);
 
diff --git a/src/util/files.h b/src/util/files.h
index fe8c00c44b..744ba93f4e 100644
--- a/src/util/files.h
+++ b/src/util/files.h
@@ -1,9 +1,9 @@
 /*
  * files.h: safer file handling
  *
+ * Copyright (C) 2010-2011 RedHat, Inc.
  * Copyright (C) 2010 IBM Corporation
  * Copyright (C) 2010 Stefan Berger
- * Copyright (C) 2010 RedHat, Inc.
  * Copyright (C) 2010 Eric Blake
  *
  * This library is free software; you can redistribute it and/or
@@ -39,7 +39,7 @@ int virFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
 FILE *virFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
 
 /* For use on normal paths; caller must check return value,
-   and failure sets errno per close(). */
+   and failure sets errno per close. */
 # define VIR_CLOSE(FD) virClose(&(FD), false)
 # define VIR_FCLOSE(FILE) virFclose(&(FILE), false)