From b2469a6d96c2385d2e5a3913d947941f2665d000 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Mon, 19 Jun 2017 12:20:45 +0100 Subject: [PATCH] common/utils: Refactor stdlib, gnulib and libxml2 cleanup functions. This refactoring change just moves the cleanup functions around in the common/utils directory. libxml2 cleanups are moved to a separate object file, so that we can still link to libutils even if the main program is not using libxml2 anywhere. Similarly gnulib cleanups. cleanup.c is renamed to cleanups.c. A new header file cleanups.h is introduced which will replace guestfs-internal-frontend-cleanups.h (fully replaced in a later commit). --- .gitignore | 3 +- common/utils/Makefile.am | 5 +- common/utils/{cleanup.c => cleanups.c} | 94 ++---------------------- common/utils/cleanups.h | 86 ++++++++++++++++++++++ common/utils/gnulib-cleanups.c | 87 ++++++++++++++++++++++ common/utils/guestfs-internal-frontend.h | 58 +-------------- common/utils/libxml2-cleanups.c | 94 ++++++++++++++++++++++++ docs/C_SOURCE_FILES | 4 +- ocaml/Makefile.am | 1 + python/Makefile.am | 15 ++-- 10 files changed, 296 insertions(+), 151 deletions(-) rename common/utils/{cleanup.c => cleanups.c} (67%) create mode 100644 common/utils/cleanups.h create mode 100644 common/utils/gnulib-cleanups.c create mode 100644 common/utils/libxml2-cleanups.c diff --git a/.gitignore b/.gitignore index 772c12b59..eb85f74c0 100644 --- a/.gitignore +++ b/.gitignore @@ -483,6 +483,8 @@ Makefile.in /python/bindtests.py /python/build /python/c-ctype.h +/python/cleanups.c +/python/cleanups.h /python/config.h /python/dist /python/examples/guestfs-python.3 @@ -491,7 +493,6 @@ Makefile.in /python/guestfs.pyc /python/guestfs.pyo /python/guestfs-internal-all.h -/python/guestfs-internal-frontend-cleanups.h /python/guestfs-internal-frontend.h /python/ignore-value.h /python/MANIFEST diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am index 93f42293a..f868da38d 100644 --- a/common/utils/Makefile.am +++ b/common/utils/Makefile.am @@ -33,9 +33,12 @@ noinst_LTLIBRARIES = libutils.la libutils_la_SOURCES = \ ../../lib/guestfs.h \ - cleanup.c \ + cleanups.c \ + cleanups.h \ + gnulib-cleanups.c \ guestfs-internal-frontend.h \ guestfs-internal-frontend-cleanups.h \ + libxml2-cleanups.c \ structs-cleanup.c \ structs-print.c \ structs-print.h \ diff --git a/common/utils/cleanup.c b/common/utils/cleanups.c similarity index 67% rename from common/utils/cleanup.c rename to common/utils/cleanups.c index 6c4558c39..cb488d9e6 100644 --- a/common/utils/cleanup.c +++ b/common/utils/cleanups.c @@ -63,37 +63,16 @@ #include #include -#include -#include -#include -#include - -#include "hash.h" - -#include "guestfs.h" #include "guestfs-internal-frontend.h" +/* Stdlib cleanups. */ + void guestfs_int_cleanup_free (void *ptr) { free (* (void **) ptr); } -void -guestfs_int_cleanup_free_string_list (char ***ptr) -{ - guestfs_int_free_string_list (*ptr); -} - -void -guestfs_int_cleanup_hash_free (void *ptr) -{ - Hash_table *h = * (Hash_table **) ptr; - - if (h) - hash_free (h); -} - void guestfs_int_cleanup_unlink_free (char **ptr) { @@ -105,69 +84,6 @@ guestfs_int_cleanup_unlink_free (char **ptr) } } -void -guestfs_int_cleanup_xmlFree (void *ptr) -{ - xmlChar *buf = * (xmlChar **) ptr; - - if (buf) - xmlFree (buf); -} - -void -guestfs_int_cleanup_xmlBufferFree (void *ptr) -{ - xmlBufferPtr xb = * (xmlBufferPtr *) ptr; - - if (xb) - xmlBufferFree (xb); -} - -void -guestfs_int_cleanup_xmlFreeDoc (void *ptr) -{ - xmlDocPtr doc = * (xmlDocPtr *) ptr; - - if (doc) - xmlFreeDoc (doc); -} - -void -guestfs_int_cleanup_xmlFreeURI (void *ptr) -{ - xmlURIPtr uri = * (xmlURIPtr *) ptr; - - if (uri) - xmlFreeURI (uri); -} - -void -guestfs_int_cleanup_xmlFreeTextWriter (void *ptr) -{ - xmlTextWriterPtr xo = * (xmlTextWriterPtr *) ptr; - - if (xo) - xmlFreeTextWriter (xo); -} - -void -guestfs_int_cleanup_xmlXPathFreeContext (void *ptr) -{ - xmlXPathContextPtr ctx = * (xmlXPathContextPtr *) ptr; - - if (ctx) - xmlXPathFreeContext (ctx); -} - -void -guestfs_int_cleanup_xmlXPathFreeObject (void *ptr) -{ - xmlXPathObjectPtr obj = * (xmlXPathObjectPtr *) ptr; - - if (obj) - xmlXPathFreeObject (obj); -} - void guestfs_int_cleanup_fclose (void *ptr) { @@ -185,3 +101,9 @@ guestfs_int_cleanup_pclose (void *ptr) if (f) pclose (f); } + +void +guestfs_int_cleanup_free_string_list (char ***ptr) +{ + guestfs_int_free_string_list (*ptr); +} diff --git a/common/utils/cleanups.h b/common/utils/cleanups.h new file mode 100644 index 000000000..488ed5642 --- /dev/null +++ b/common/utils/cleanups.h @@ -0,0 +1,86 @@ +/* libguestfs + * Copyright (C) 2013-2017 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 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, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef GUESTFS_CLEANUPS_H_ +#define GUESTFS_CLEANUPS_H_ + +#ifdef HAVE_ATTRIBUTE_CLEANUP +#define CLEANUP_FREE \ + __attribute__((cleanup(guestfs_int_cleanup_free))) +#define CLEANUP_HASH_FREE \ + __attribute__((cleanup(guestfs_int_cleanup_hash_free))) +#define CLEANUP_GL_RECURSIVE_LOCK_UNLOCK \ + __attribute__((cleanup(guestfs_int_cleanup_gl_recursive_lock_unlock))) +#define CLEANUP_UNLINK_FREE \ + __attribute__((cleanup(guestfs_int_cleanup_unlink_free))) +#define CLEANUP_FCLOSE \ + __attribute__((cleanup(guestfs_int_cleanup_fclose))) +#define CLEANUP_PCLOSE \ + __attribute__((cleanup(guestfs_int_cleanup_pclose))) +#define CLEANUP_FREE_STRING_LIST \ + __attribute__((cleanup(guestfs_int_cleanup_free_string_list))) +#define CLEANUP_XMLFREE \ + __attribute__((cleanup(guestfs_int_cleanup_xmlFree))) +#define CLEANUP_XMLBUFFERFREE \ + __attribute__((cleanup(guestfs_int_cleanup_xmlBufferFree))) +#define CLEANUP_XMLFREEDOC \ + __attribute__((cleanup(guestfs_int_cleanup_xmlFreeDoc))) +#define CLEANUP_XMLFREEURI \ + __attribute__((cleanup(guestfs_int_cleanup_xmlFreeURI))) +#define CLEANUP_XMLFREETEXTWRITER \ + __attribute__((cleanup(guestfs_int_cleanup_xmlFreeTextWriter))) +#define CLEANUP_XMLXPATHFREECONTEXT \ + __attribute__((cleanup(guestfs_int_cleanup_xmlXPathFreeContext))) +#define CLEANUP_XMLXPATHFREEOBJECT \ + __attribute__((cleanup(guestfs_int_cleanup_xmlXPathFreeObject))) +#else +#define CLEANUP_FREE +#define CLEANUP_HASH_FREE +/* XXX no safe equivalent to CLEANUP_GL_RECURSIVE_LOCK_UNLOCK */ +#define CLEANUP_UNLINK_FREE +#define CLEANUP_FCLOSE +#define CLEANUP_PCLOSE +#define CLEANUP_FREE_STRING_LIST +#define CLEANUP_XMLFREE +#define CLEANUP_XMLBUFFERFREE +#define CLEANUP_XMLFREEDOC +#define CLEANUP_XMLFREEURI +#define CLEANUP_XMLFREETEXTWRITER +#define CLEANUP_XMLXPATHFREECONTEXT +#define CLEANUP_XMLXPATHFREEOBJECT +#endif + +/* These functions are used internally by the CLEANUP_* macros. + * Don't call them directly. + */ +extern void guestfs_int_cleanup_free (void *ptr); +extern void guestfs_int_cleanup_hash_free (void *ptr); +extern void guestfs_int_cleanup_gl_recursive_lock_unlock (void *ptr); +extern void guestfs_int_cleanup_unlink_free (char **ptr); +extern void guestfs_int_cleanup_fclose (void *ptr); +extern void guestfs_int_cleanup_pclose (void *ptr); +extern void guestfs_int_cleanup_free_string_list (char ***ptr); +extern void guestfs_int_cleanup_xmlFree (void *ptr); +extern void guestfs_int_cleanup_xmlBufferFree (void *ptr); +extern void guestfs_int_cleanup_xmlFreeDoc (void *ptr); +extern void guestfs_int_cleanup_xmlFreeURI (void *ptr); +extern void guestfs_int_cleanup_xmlFreeTextWriter (void *ptr); +extern void guestfs_int_cleanup_xmlXPathFreeContext (void *ptr); +extern void guestfs_int_cleanup_xmlXPathFreeObject (void *ptr); + +#endif /* GUESTFS_CLEANUPS_H_ */ diff --git a/common/utils/gnulib-cleanups.c b/common/utils/gnulib-cleanups.c new file mode 100644 index 000000000..51c6691e1 --- /dev/null +++ b/common/utils/gnulib-cleanups.c @@ -0,0 +1,87 @@ +/* libguestfs + * Copyright (C) 2013-2017 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 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, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * Libguestfs uses C macros to simplify temporary + * allocations. They are implemented using the + * C<__attribute__((cleanup))> feature of gcc and clang. Typical + * usage is: + * + * fn () + * { + * CLEANUP_FREE char *str = NULL; + * str = safe_asprintf (g, "foo"); + * // str is freed automatically when the function returns + * } + * + * There are a few catches to be aware of with the cleanup mechanism: + * + * =over 4 + * + * =item * + * + * If a cleanup variable is not initialized, then you can end up + * calling L with an undefined value, resulting in the + * program crashing. For this reason, you should usually initialize + * every cleanup variable with something, eg. C + * + * =item * + * + * Don't mark variables holding return values as cleanup variables. + * + * =item * + * + * The C function shouldn't use cleanup variables since it is + * normally exited by calling L, and that doesn't call the + * cleanup handlers. + * + * =back + * + * The functions in this file are used internally by the C + * macros. Don't call them directly. + */ + +#include + +#include +#include +#include +#include + +#include "guestfs-internal-frontend.h" + +#include "glthread/lock.h" +#include "hash.h" + +/* Gnulib cleanups. */ + +void +guestfs_int_cleanup_gl_recursive_lock_unlock (void *ptr) +{ + gl_recursive_lock_t *lockp = * (gl_recursive_lock_t **) ptr; + gl_recursive_lock_unlock (*lockp); +} + +void +guestfs_int_cleanup_hash_free (void *ptr) +{ + Hash_table *h = * (Hash_table **) ptr; + + if (h) + hash_free (h); +} diff --git a/common/utils/guestfs-internal-frontend.h b/common/utils/guestfs-internal-frontend.h index 489b54ffd..1d5a50664 100644 --- a/common/utils/guestfs-internal-frontend.h +++ b/common/utils/guestfs-internal-frontend.h @@ -35,50 +35,11 @@ #include #include "guestfs-internal-all.h" +#include "cleanups.h" #define _(str) dgettext(PACKAGE, (str)) #define N_(str) dgettext(PACKAGE, (str)) -#ifdef HAVE_ATTRIBUTE_CLEANUP -#define CLEANUP_FREE __attribute__((cleanup(guestfs_int_cleanup_free))) -#define CLEANUP_FREE_STRING_LIST \ - __attribute__((cleanup(guestfs_int_cleanup_free_string_list))) -#define CLEANUP_HASH_FREE \ - __attribute__((cleanup(guestfs_int_cleanup_hash_free))) -#define CLEANUP_UNLINK_FREE \ - __attribute__((cleanup(guestfs_int_cleanup_unlink_free))) -#define CLEANUP_XMLFREE \ - __attribute__((cleanup(guestfs_int_cleanup_xmlFree))) -#define CLEANUP_XMLBUFFERFREE \ - __attribute__((cleanup(guestfs_int_cleanup_xmlBufferFree))) -#define CLEANUP_XMLFREEDOC \ - __attribute__((cleanup(guestfs_int_cleanup_xmlFreeDoc))) -#define CLEANUP_XMLFREEURI \ - __attribute__((cleanup(guestfs_int_cleanup_xmlFreeURI))) -#define CLEANUP_XMLFREETEXTWRITER \ - __attribute__((cleanup(guestfs_int_cleanup_xmlFreeTextWriter))) -#define CLEANUP_XMLXPATHFREECONTEXT \ - __attribute__((cleanup(guestfs_int_cleanup_xmlXPathFreeContext))) -#define CLEANUP_XMLXPATHFREEOBJECT \ - __attribute__((cleanup(guestfs_int_cleanup_xmlXPathFreeObject))) -#define CLEANUP_FCLOSE __attribute__((cleanup(guestfs_int_cleanup_fclose))) -#define CLEANUP_PCLOSE __attribute__((cleanup(guestfs_int_cleanup_pclose))) -#else -#define CLEANUP_FREE -#define CLEANUP_FREE_STRING_LIST -#define CLEANUP_HASH_FREE -#define CLEANUP_UNLINK_FREE -#define CLEANUP_XMLFREE -#define CLEANUP_XMLBUFFERFREE -#define CLEANUP_XMLFREEDOC -#define CLEANUP_XMLFREEURI -#define CLEANUP_XMLFREETEXTWRITER -#define CLEANUP_XMLXPATHFREECONTEXT -#define CLEANUP_XMLXPATHFREEOBJECT -#define CLEANUP_FCLOSE -#define CLEANUP_PCLOSE -#endif - /* utils.c */ extern void guestfs_int_free_string_list (char **); extern size_t guestfs_int_count_strings (char *const *); @@ -102,23 +63,6 @@ extern void guestfs_int_fadvise_noreuse (int fd); //extern void guestfs_int_fadvise_willneed (int fd); extern char *guestfs_int_shell_unquote (const char *str); -/* These functions are used internally by the CLEANUP_* macros. - * Don't call them directly. - */ -extern void guestfs_int_cleanup_free (void *ptr); -extern void guestfs_int_cleanup_free_string_list (char ***ptr); -extern void guestfs_int_cleanup_hash_free (void *ptr); -extern void guestfs_int_cleanup_unlink_free (char **ptr); -extern void guestfs_int_cleanup_xmlFree (void *ptr); -extern void guestfs_int_cleanup_xmlBufferFree (void *ptr); -extern void guestfs_int_cleanup_xmlFreeDoc (void *ptr); -extern void guestfs_int_cleanup_xmlFreeURI (void *ptr); -extern void guestfs_int_cleanup_xmlFreeTextWriter (void *ptr); -extern void guestfs_int_cleanup_xmlXPathFreeContext (void *ptr); -extern void guestfs_int_cleanup_xmlXPathFreeObject (void *ptr); -extern void guestfs_int_cleanup_fclose (void *ptr); -extern void guestfs_int_cleanup_pclose (void *ptr); - /* These are in a separate header so the header can be generated. * Don't include the following file directly: */ diff --git a/common/utils/libxml2-cleanups.c b/common/utils/libxml2-cleanups.c new file mode 100644 index 000000000..066573fef --- /dev/null +++ b/common/utils/libxml2-cleanups.c @@ -0,0 +1,94 @@ +/* libguestfs + * Copyright (C) 2013-2017 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 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, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include + +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "cleanups.h" + +void +guestfs_int_cleanup_xmlFree (void *ptr) +{ + xmlChar *buf = * (xmlChar **) ptr; + + if (buf) + xmlFree (buf); +} + +void +guestfs_int_cleanup_xmlBufferFree (void *ptr) +{ + xmlBufferPtr xb = * (xmlBufferPtr *) ptr; + + if (xb) + xmlBufferFree (xb); +} + +void +guestfs_int_cleanup_xmlFreeDoc (void *ptr) +{ + xmlDocPtr doc = * (xmlDocPtr *) ptr; + + if (doc) + xmlFreeDoc (doc); +} + +void +guestfs_int_cleanup_xmlFreeURI (void *ptr) +{ + xmlURIPtr uri = * (xmlURIPtr *) ptr; + + if (uri) + xmlFreeURI (uri); +} + +void +guestfs_int_cleanup_xmlFreeTextWriter (void *ptr) +{ + xmlTextWriterPtr xo = * (xmlTextWriterPtr *) ptr; + + if (xo) + xmlFreeTextWriter (xo); +} + +void +guestfs_int_cleanup_xmlXPathFreeContext (void *ptr) +{ + xmlXPathContextPtr ctx = * (xmlXPathContextPtr *) ptr; + + if (ctx) + xmlXPathFreeContext (ctx); +} + +void +guestfs_int_cleanup_xmlXPathFreeObject (void *ptr) +{ + xmlXPathObjectPtr obj = * (xmlXPathObjectPtr *) ptr; + + if (obj) + xmlXPathFreeObject (obj); +} diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES index ccb3a091a..f690fb473 100644 --- a/docs/C_SOURCE_FILES +++ b/docs/C_SOURCE_FILES @@ -42,9 +42,11 @@ common/progress/progress.h common/qemuopts/qemuopts-tests.c common/qemuopts/qemuopts.c common/qemuopts/qemuopts.h -common/utils/cleanup.c +common/utils/cleanups.c +common/utils/cleanups.h common/utils/guestfs-internal-frontend-cleanups.h common/utils/guestfs-internal-frontend.h +common/utils/libxml2-cleanups.c common/utils/structs-cleanup.c common/utils/structs-print.c common/utils/structs-print.h diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am index 57ecd608b..91be080bf 100644 --- a/ocaml/Makefile.am +++ b/ocaml/Makefile.am @@ -94,6 +94,7 @@ libguestfsocaml_a_SOURCES = \ guestfs-c.c \ guestfs-c-actions.c \ guestfs-c-errnos.c \ + ../common/utils/cleanups.c \ ../common/utils/utils.c if HAVE_OCAMLDOC diff --git a/python/Makefile.am b/python/Makefile.am index ae90aa01d..7e91bbb7e 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -97,9 +97,10 @@ setup-install: setup.py stamp-extra-files # to hard-link any extra files we need into the local directory. stamp-extra-files: \ c-ctype.h \ + cleanups.c \ + cleanups.h \ config.h \ guestfs-internal-all.h \ - guestfs-internal-frontend-cleanups.h \ guestfs-internal-frontend.h \ ignore-value.h \ utils.c @@ -111,15 +112,18 @@ config.h: c-ctype.h: ln $(top_srcdir)/gnulib/lib/c-ctype.h $@ +cleanups.c: + ln $(top_srcdir)/common/utils/cleanups.c $@ + +cleanups.h: + ln $(top_srcdir)/common/utils/cleanups.h $@ + ignore-value.h: ln $(top_srcdir)/gnulib/lib/ignore-value.h $@ guestfs-internal-all.h: ln $(top_srcdir)/lib/guestfs-internal-all.h $@ -guestfs-internal-frontend-cleanups.h: - ln $(top_srcdir)/common/utils/guestfs-internal-frontend-cleanups.h $@ - guestfs-internal-frontend.h: ln $(top_srcdir)/common/utils/guestfs-internal-frontend.h $@ @@ -145,8 +149,9 @@ CLEANFILES += \ t/*~ t/*.pyc \ c-ctype.h \ config.h \ + cleanups.c \ + cleanups.h \ guestfs-internal-all.h \ - guestfs-internal-frontend-cleanups.h \ guestfs-internal-frontend.h \ ignore-value.h \ stamp-extra-files \