[lttng-dev] [Babeltrace RFC PATCH 03/28] Move build-specific strerror_r to compat directory
Ikaheimonen, JP
jp_ikaheimonen at mentor.com
Thu May 16 06:01:06 EDT 2013
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:mathieu.desnoyers at efficios.com]
Sent: 14. toukokuuta 2013 15:09
To: Jérémie Galarneau
Cc: Ikaheimonen, JP; lttng-dev at lists.lttng.org
Subject: Re: [lttng-dev] [Babeltrace RFC PATCH 03/28] Move build-specific strerror_r to compat directory
* Jérémie Galarneau (jeremie.galarneau at efficios.com) wrote:
> On Thu, May 2, 2013 at 7:46 AM, Ikaheimonen, JP
> <jp_ikaheimonen at mentor.com> 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
> > lttng-dev at lists.lttng.org
> > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
>
>
> --
> Jérémie Galarneau
> EfficiOS Inc.
> http://www.efficios.com
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list