Mathieu, Jeremie: thank you for your comments. I will prepare a new set of patches, and post them sometime early next week.
Thanks again, JP Ikaheimonen Mentor Graphics -----Original Message----- From: Mathieu Desnoyers [mailto:[email protected]] Sent: 14. toukokuuta 2013 15:09 To: Jérémie Galarneau Cc: Ikaheimonen, JP; [email protected] Subject: Re: [lttng-dev] [Babeltrace RFC PATCH 03/28] Move build-specific strerror_r to compat directory * Jérémie Galarneau ([email protected]) wrote: > On Thu, May 2, 2013 at 7:46 AM, Ikaheimonen, JP > <[email protected]> wrote: > > Add a new source directory 'compat'. > > In that directory, add a new file 'strlib.c'. > > In that file, add a new function compat_strerror_r that encapsulates > > the build-specific differences of strerror_r. > > Where strerror_r is used, now use compat_strerror_r. > > --- > > Makefile.am | 2 +- > > compat/Makefile.am | 10 ++++++++++ > > compat/compat_strlib.c | 17 +++++++++++++++++ > > configure.ac | 1 + > > include/babeltrace/babeltrace-internal.h | 32 > > +++----------------------------- > > include/babeltrace/compat/string.h | 9 +++++++++ > > lib/Makefile.am | 3 ++- > > 7 files changed, 43 insertions(+), 31 deletions(-) create mode > > 100644 compat/Makefile.am create mode 100644 compat/compat_strlib.c > > create mode 100644 include/babeltrace/compat/string.h > > > > diff --git a/Makefile.am b/Makefile.am index b25b58f..cdfad5a 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -2,7 +2,7 @@ AM_CFLAGS = $(PACKAGE_CFLAGS) > > -I$(top_srcdir)/include > > > > ACLOCAL_AMFLAGS = -I m4 > > > > -SUBDIRS = include types lib formats converter tests doc extras > > +SUBDIRS = include types compat lib formats converter tests doc > > +extras > > > > dist_doc_DATA = ChangeLog LICENSE mit-license.txt gpl-2.0.txt \ > > std-ext-lib.txt > > diff --git a/compat/Makefile.am b/compat/Makefile.am new file mode > > 100644 index 0000000..d756aa7 > > --- /dev/null > > +++ b/compat/Makefile.am > > @@ -0,0 +1,10 @@ > > +AM_CFLAGS = $(PACKAGE_CFLAGS) -I$(top_srcdir)/include > > + > > +lib_LTLIBRARIES = libcompat.la > > + > > +libcompat_la_SOURCES = \ > > + compat_strlib.c > > + > > +libcompat_la_LDFLAGS = \ > > + -Wl,--no-as-needed > > + > > diff --git a/compat/compat_strlib.c b/compat/compat_strlib.c new > > file mode 100644 index 0000000..9aabb07 > > --- /dev/null > > +++ b/compat/compat_strlib.c > > @@ -0,0 +1,17 @@ > > +#include <babeltrace/compat/string.h> > > + > > +int compat_strerror_r(int errnum, char *buf, size_t buflen) > > Is there a specific reason why you can't redefine this as a macro > instead of changing the function's name? whenever possible, we try to use static inline rather than macros. This provides much better type verification, and cleaner compiler error reporting. however, a big note on how to make clean wrappers: Do: #if something int myfct(args) { ... } #else int myfct(args) { ... } #endif and _not_ the glibc-style oddness: int myfct(args) { #if something ... #else ... #endif } putting preprocessor conditions within functions is purely evil. ;-) Thanks, Mathieu > > > +{ > > +#if !defined(__linux__) || ((_POSIX_C_SOURCE >= 200112L || > > +_XOPEN_SOURCE >= 600) && !defined(_GNU_SOURCE)) > > +/* XSI-compliant strerror_r */ > > + return strerror_r(errnum, buf, buflen); #else > > +/* GNU-compliant strerror_r */ > > + char * retbuf; > > + retbuf = strerror_r(errnum, buf, buflen); > > + if (retbuf != buf) > > + strcpy(buf, retbuf); > > + return 0; > > +#endif > > +} > > + > > diff --git a/configure.ac b/configure.ac index 83822d6..29366da > > 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -93,6 +93,7 @@ AC_SUBST(babeltracectfincludedir) > > AC_CONFIG_FILES([ > > Makefile > > types/Makefile > > + compat/Makefile > > formats/Makefile > > formats/ctf/Makefile > > formats/ctf/types/Makefile > > diff --git a/include/babeltrace/babeltrace-internal.h > > b/include/babeltrace/babeltrace-internal.h > > index 22866bc..35382de 100644 > > --- a/include/babeltrace/babeltrace-internal.h > > +++ b/include/babeltrace/babeltrace-internal.h > > @@ -27,7 +27,7 @@ > > #include <stdio.h> > > #include <glib.h> > > #include <stdint.h> > > -#include <string.h> > > +#include <babeltrace/compat/string.h> > > > > #define PERROR_BUFLEN 200 > > > > @@ -81,46 +81,20 @@ extern int babeltrace_verbose, babeltrace_debug; > > perrorstr, \ > > ## args) > > > > -#if !defined(__linux__) || ((_POSIX_C_SOURCE >= 200112L || > > _XOPEN_SOURCE >= 600) && !defined(_GNU_SOURCE)) > > - > > #define _bt_printf_perror(fp, fmt, args...) \ > > ({ \ > > char buf[PERROR_BUFLEN] = "Error in strerror_r()"; \ > > - strerror_r(errno, buf, sizeof(buf)); \ > > + compat_strerror_r(errno, buf, sizeof(buf)); > > \ > > _bt_printfe(fp, "error", buf, fmt, ## args); \ > > }) > > > > #define _bt_printfl_perror(fp, lineno, fmt, args...) \ > > ({ \ > > char buf[PERROR_BUFLEN] = "Error in strerror_r()"; \ > > - strerror_r(errno, buf, sizeof(buf)); \ > > + compat_strerror_r(errno, buf, sizeof(buf)); > > \ > > _bt_printfle(fp, "error", lineno, buf, fmt, ## args); \ > > }) > > > > -#else > > - > > -/* > > - * Version using GNU strerror_r, for linux with appropriate defines. > > - */ > > - > > -#define _bt_printf_perror(fp, fmt, args...) \ > > - ({ \ > > - char *buf; \ > > - char tmp[PERROR_BUFLEN] = "Error in strerror_r()"; \ > > - buf = strerror_r(errno, tmp, sizeof(tmp)); \ > > - _bt_printfe(fp, "error", buf, fmt, ## args); \ > > - }) > > - > > -#define _bt_printfl_perror(fp, lineno, fmt, args...) \ > > - ({ \ > > - char *buf; \ > > - char tmp[PERROR_BUFLEN] = "Error in strerror_r()"; \ > > - buf = strerror_r(errno, tmp, sizeof(tmp)); \ > > - _bt_printfle(fp, "error", lineno, buf, fmt, ## args); \ > > - }) > > - > > -#endif > > - > > /* printf without lineno information */ > > #define printf_fatal(fmt, args...) \ > > _bt_printf(stderr, "fatal", fmt, ## args) diff --git > > a/include/babeltrace/compat/string.h > > b/include/babeltrace/compat/string.h > > new file mode 100644 > > index 0000000..8591b7e > > --- /dev/null > > +++ b/include/babeltrace/compat/string.h > > @@ -0,0 +1,9 @@ > > +#ifndef _BABELTRACE_COMPAT_STRING_H #define > > +BABELTRACE_COMPAT_STRING_H > > + > > +#include <string.h> > > + > > +int compat_strerror_r(int errnum, char *buf, size_t buflen); > > + > > +#endif > > + > > diff --git a/lib/Makefile.am b/lib/Makefile.am index > > 7ffb164..fa470c0 100644 > > --- a/lib/Makefile.am > > +++ b/lib/Makefile.am > > @@ -14,4 +14,5 @@ libbabeltrace_la_SOURCES = babeltrace.c \ > > libbabeltrace_la_LDFLAGS = \ > > -Wl,--no-as-needed \ > > prio_heap/libprio_heap.la \ > > - $(top_builddir)/types/libbabeltrace_types.la > > + $(top_builddir)/types/libbabeltrace_types.la \ > > + $(top_builddir)/compat/libcompat.la > > -- > > 1.8.1.msysgit.1 > > > > > > _______________________________________________ > > lttng-dev mailing list > > [email protected] > > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > > > > -- > Jérémie Galarneau > EfficiOS Inc. > http://www.efficios.com > > _______________________________________________ > lttng-dev mailing list > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
