[Qemu-devel] [PATCH RFC 09/10] maint: add check for use of POSIX functions which are not reentrant safe

[Daniel P. Berrange]

Imports a rule from libvirt to make sure that for any POSIX
function which is not reentrant safe, the _r variant is used
in the source.

While it is entirely possible that many of the QEMU uses are
in fact safe, it is pretty difficult to prove that conclusively
due to the increasing use of 3rd party libraries. For features
like spice, glusterfs, rbd, iscsi these libraries may either be
used from QEMU non-main threads, or can be  secretly using threads
themselves behind QEMU's back.

Given this, the only safe thing todo is to forbid all use of the
non-reentrant safe POSIX functions. While Linux platforms have
long had the full set of _r variants, other OS may not be so lucky,
particularly Mingw32, so fixing this will require some portability
code on various platforms.

Since fixing the current QEMU usage is non-trivial, the check is
disabled for any functions QEMU currently relies on. IOW, this
check merely stops the current problem getting any worse. Future
work will have to look at fixing existing violations.

Signed-off-by: Daniel P. Berrange <address@hidden>
---
 Makefile.nonreentrant | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++
 cfg.mk                |  14 ++++++
 2 files changed, 134 insertions(+)
 create mode 100644 Makefile.nonreentrant

diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant
new file mode 100644
index 0000000..687d39b
--- /dev/null
+++ b/Makefile.nonreentrant
@@ -0,0 +1,120 @@
+## Functions for sc_prohibit_nonreentrant  -*- makefile -*-
+##
+## Copyright (C) 2009-2010, 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
+## License as published by the Free Software Foundation; either
+## version 2.1 of the License, or (at your option) any later version.
+##
+## This library is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+## Lesser General Public License for more details.
+##
+## You should have received a copy of the GNU Lesser General Public
+## License along with this library.  If not, see
+## <http://www.gnu.org/licenses/>.
+
+#
+# Generated by running the following on Fedora 9:
+#
+#  nm -D --defined-only /lib/libc.so.6  \
+#      | grep '_r$' \
+#      | awk '{print $3}' \
+#      | grep -v __ \
+#      | grep -v qsort \ # Red herring since we don't need to pass extra args 
to qsort comparator
+#      | grep -v readdir \ # This is safe as long as each DIR * instance is 
only used by one thread
+#      | sort \
+#      | uniq \
+#      | sed -e 's/_r//'
+#
+# Also manually add in all inet_* functions some of which
+# are not threadsafe and do not have _r variants. They are
+# all deprecated in favour of getnameinfo/getaddrinfo
+#
+
+# Current QEMU violations are commented out. Eventual goal
+# is to uncomment all these functions, once QEMU code is
+# fixed
+NON_REENTRANT =
+NON_REENTRANT += asctime
+NON_REENTRANT += ctime
+NON_REENTRANT += drand48
+NON_REENTRANT += ecvt
+NON_REENTRANT += erand48
+NON_REENTRANT += ether_aton
+NON_REENTRANT += ether_ntoa
+#NON_REENTRANT += fcvt
+NON_REENTRANT += fgetgrent
+NON_REENTRANT += fgetpwent
+NON_REENTRANT += fgetspent
+NON_REENTRANT += getaliasbyname
+NON_REENTRANT += getaliasent
+NON_REENTRANT += getdate
+NON_REENTRANT += getgrent
+NON_REENTRANT += getgrgid
+NON_REENTRANT += getgrnam
+NON_REENTRANT += gethostbyaddr
+NON_REENTRANT += gethostbyname2
+#NON_REENTRANT += gethostbyname
+NON_REENTRANT += gethostent
+NON_REENTRANT += getlogin
+#NON_REENTRANT += getmntent
+NON_REENTRANT += getnetbyaddr
+NON_REENTRANT += getnetbyname
+NON_REENTRANT += getnetent
+NON_REENTRANT += getnetgrent
+NON_REENTRANT += getprotobyname
+NON_REENTRANT += getprotobynumber
+NON_REENTRANT += getprotoent
+NON_REENTRANT += getpwent
+#NON_REENTRANT += getpwnam
+#NON_REENTRANT += getpwuid
+NON_REENTRANT += getrpcbyname
+NON_REENTRANT += getrpcbynumber
+NON_REENTRANT += getrpcent
+NON_REENTRANT += getservbyname
+NON_REENTRANT += getservbyport
+NON_REENTRANT += getservent
+NON_REENTRANT += getspent
+NON_REENTRANT += getspnam
+NON_REENTRANT += getutent
+NON_REENTRANT += getutid
+NON_REENTRANT += getutline
+#NON_REENTRANT += gmtime
+NON_REENTRANT += hcreate
+NON_REENTRANT += hdestroy
+NON_REENTRANT += hsearch
+NON_REENTRANT += initstate
+NON_REENTRANT += jrand48
+NON_REENTRANT += lcong48
+#NON_REENTRANT += localtime
+NON_REENTRANT += lrand48
+NON_REENTRANT += mrand48
+NON_REENTRANT += nrand48
+#NON_REENTRANT += ptsname
+NON_REENTRANT += qecvt
+NON_REENTRANT += qfcvt
+#NON_REENTRANT += random
+#NON_REENTRANT += rand
+NON_REENTRANT += seed48
+NON_REENTRANT += setstate
+NON_REENTRANT += sgetspent
+NON_REENTRANT += srand48
+#NON_REENTRANT += srandom
+#NON_REENTRANT += strerror
+#NON_REENTRANT += strtok
+NON_REENTRANT += tmpnam
+NON_REENTRANT += ttyname
+#NON_REENTRANT += inet_addr
+#NON_REENTRANT += inet_aton
+NON_REENTRANT += inet_lnaof
+NON_REENTRANT += inet_makeaddr
+NON_REENTRANT += inet_netof
+NON_REENTRANT += inet_network
+NON_REENTRANT += inet_nsap_addr
+NON_REENTRANT += inet_nsap_ntoa
+#NON_REENTRANT += inet_ntoa
+#NON_REENTRANT += inet_ntop
+#NON_REENTRANT += inet_pton
diff --git a/cfg.mk b/cfg.mk
index 39e4124..2f98c1a 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -128,6 +128,20 @@ sc_copyright_format:
        halt='spell Red Hat as two words'                               \
          $(_sc_search_regexp)
 
+# Use a subshell for each function, to give the optimal warning message.
+include $(srcdir)/Makefile.nonreentrant
+
+sc_prohibit_nonreentrant:
+       @for i in $(NON_REENTRANT) ; \
+        do \
+            (prohibit="\\<$$i *\\("                                     \
+             in_vc_files='\.[ch]$$'                                    \
+             halt="use $${i}_r, not $$i"                                \
+             $(_sc_search_regexp)                                       \
+            ) || fail=1;                                                \
+        done ; \
+        exit $$fail
+
 # We don't use this feature of maint.mk.
 prev_version_file = /dev/null
 
-- 
2.4.3