Re: [libvirt] PATCH: 15/25: Prohibit non-threadsafe POSIX apis

There are a huge list of functions in POSIX which are not safe to use from multiple threads currently. I generated the list by looking at all libc symbol exports for variants which have a parallel _r symbol. nm -D --defined-only /lib/libc.so.6 \ | grep '_r$' \ | awk '{print $3}' \ | grep -v __ \ | grep -v qsort \ | grep -v readdir \ | sort \ | uniq \ | sed -e 's/_r//' The qsort one is a red herring, since you only need qsort_r if you need to pass a extra 'void * opaque' data blob to your sort function - we don't, so don't need qsort_r. The readdir one is also unneccessary, since reading from a single DIR* is safe from a single thread. readdir_r is also horrific http://womble.decadentplace.org.uk/readdir_r-advisory.html This patch adds a 'make sc_prohibit_nonrentrant' rule to the 'syntax-check' for these forbidden functions. .x-sc_prohibit_nonreentrant | 8 ++++ Makefile.am | 2 + Makefile.maint | 11 +++++ Makefile.nonreentrant | 85 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+) Daniel diff --git a/.x-sc_prohibit_nonreentrant b/.x-sc_prohibit_nonreentrant new file mode 100644 --- /dev/null +++ b/.x-sc_prohibit_nonreentrant @@ -0,0 +1,8 @@ +^gnulib/ +^po/ +ChangeLog +^Makefile* +^docs/ +^tests/ +^src/virsh.c +^build-aux/ diff --git a/Makefile.am b/Makefile.am --- a/Makefile.am +++ b/Makefile.am @@ -17,6 +17,8 @@ EXTRA_DIST = \ .x-sc_require_config_h_first \ .x-sc_prohibit_strcmp \ .x-sc_require_config_h \ + .x-sc_prohibit_nonreentrant \ + Makefile.nonreentrant \ autogen.sh man_MANS = virsh.1 diff --git a/Makefile.maint b/Makefile.maint --- a/Makefile.maint +++ b/Makefile.maint @@ -12,6 +12,8 @@ VC_LIST = $(srcdir)/build-aux/vc-list-fi VC_LIST_EXCEPT = \ $(VC_LIST) | if test -f .x-$@; then grep -vEf .x-$@; else grep -v ChangeLog; fi + +include Makefile.nonreentrant # Prevent programs like 'sort' from considering distinct strings to be equal. # Doing it here saves us from having to set LC_ALL elsewhere in this file. @@ -110,6 +112,15 @@ sc_prohibit_asprintf: sc_prohibit_asprintf: @grep -nE '\<[a]sprintf\>' $$($(VC_LIST_EXCEPT)) && \ { echo '$(ME): use virAsprintf, not a'sprintf 1>&2; exit 1; } || : + +sc_prohibit_nonreentrant: + @fail=0 ; \ + for i in $(NON_REENTRANT) ; \ + do \ + grep -nE "\<$$i\>[:space:]*\(" $$($(VC_LIST_EXCEPT)) && \ + fail=1 && echo "$(ME): use $${i}_r, not $${i}" || : ; \ + done ; \ + exit $$fail # Using EXIT_SUCCESS as the first argument to error is misleading, # since when that parameter is 0, error does not exit. Use `0' instead. diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant new file mode 100644 --- /dev/null +++ b/Makefile.nonreentrant @@ -0,0 +1,85 @@ + +# +# 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//' +# + +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 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|