[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Lightning] config.h should not be included in the library header
From: |
Marc Nieper-Wißkirchen |
Subject: |
Re: [Lightning] config.h should not be included in the library header |
Date: |
Thu, 29 Aug 2019 10:20:57 +0200 |
Dear Paulo,
here is a patch. The actual code seen by the compiler didn't change so
it is safe even for some ancient targets:
--- PATCH BEGIN ---
diff --git a/ChangeLog b/ChangeLog
index 150af31..3b445a1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2019-08-29 Marc Nieper-Wißkirchen <address@hidden>
+
+ * include/lightning/jit_private.h: Move definition of offsetof
+ from the public header file here.
+
+ * configure.ac, include/Makefile.am, include/lightning.h,
+ include/lightning.h.in: Generate lightning.h from lightning.in.h
+ and remove the dependence on config.h from the public header file.
+
2019-06-04 Paulo Andrade <address@hidden>
* include/lightning/jit_riscv.h, lib/jit_riscv-cpu.c,
diff --git a/configure.ac b/configure.ac
index f973b53..ba28ce4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -285,10 +285,15 @@ AC_SUBST(cpu)
AC_SUBST([LIGHTNING_CFLAGS])
+if test $ac_cv_header_stdint_h = yes; then
+ AC_SUBST(MAYBE_INCLUDE_STDINT_H, ["#include <stdint.h>"])
+fi
+
AC_OUTPUT([Makefile
lightning.pc
doc/Makefile
- include/Makefile
+ include/Makefile
include/lightning/Makefile
- lib/Makefile
+ include/lightning.h
+ lib/Makefile
check/Makefile])
diff --git a/doc/version.texi b/doc/version.texi
index 1d47234..44b229d 100644
--- a/doc/version.texi
+++ b/doc/version.texi
@@ -1,4 +1,4 @@
-@set UPDATED 4 September 2017
-@set UPDATED-MONTH September 2017
-@set EDITION 2.1.1
-@set VERSION 2.1.1
+@set UPDATED 29 August 2019
+@set UPDATED-MONTH August 2019
+@set EDITION 2.1.2
+@set VERSION 2.1.2
diff --git a/include/Makefile.am b/include/Makefile.am
index 601ae7a..7914e31 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -17,3 +17,5 @@
include_HEADERS = lightning.h
SUBDIRS = \
lightning
+
+nodist_include_HEADERS = lightning.h
diff --git a/include/lightning.h b/include/lightning.h.in
similarity index 99%
rename from include/lightning.h
rename to include/lightning.h.in
index 8a142a2..9030fa8 100644
--- a/include/lightning.h
+++ b/include/lightning.h.in
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012-2017 Free Software Foundation, Inc.
+ * Copyright (C) 2012-2017, 2019 Free Software Foundation, Inc.
*
* This file is part of GNU lightning.
*
@@ -20,15 +20,9 @@
#ifndef _lightning_h
#define _lightning_h
-#if HAVE_CONFIG_H
-# include "config.h"
-#endif
-
#include <unistd.h>
#include <stdlib.h>
-#if HAVE_STDINT_H
-# include <stdint.h>
-#endif
+@MAYBE_INCLUDE_STDINT_H@
#include <string.h>
#if defined(__hpux) && defined(__hppa__)
@@ -38,14 +32,6 @@
# include <machine/endian.h>
#endif
-#ifdef STDC_HEADERS
-# include <stddef.h>
-#else
-# if !defined(offsetof)
-# define offsetof(type, field) ((char *)&((type *)0)->field - (char *)0)
-# endif
-#endif
-
#ifndef __WORDSIZE
# if defined(WORDSIZE) /* ppc darwin */
# define __WORDSIZE WORDSIZE
diff --git a/include/lightning/jit_private.h b/include/lightning/jit_private.h
index 9e0a8be..492d070 100644
--- a/include/lightning/jit_private.h
+++ b/include/lightning/jit_private.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012-2017 Free Software Foundation, Inc.
+ * Copyright (C) 2012-2017, 2019 Free Software Foundation, Inc.
*
* This file is part of GNU lightning.
*
@@ -28,6 +28,14 @@
#include <limits.h>
#include <stdio.h>
+#ifdef STDC_HEADERS
+# include <stddef.h>
+#else
+# if !defined(offsetof)
+# define offsetof(type, field) ((char *)&((type *)0)->field - (char *)0)
+# endif
+#endif
+
#if defined(__GNUC__)
# define maybe_unused __attribute__ ((unused))
# define unlikely(exprn) __builtin_expect(!!(exprn), 0)
--- PATCH END ---
What I haven't changed yet (but should be considered to be changed in
some future version of lightning) is renaming of "__WORDSIZE",
"__LITTLE_ENDIAN", and so forth, to something like "JIT_WORDSIZE",
"JIT_LITTLE_ENDIAN" so that we don't mess with reserved namespaces.
(There may be more "__"-identifiers occurring in the machine-specific
include files.
Best,
Marc
Am Mi., 28. Aug. 2019 um 20:42 Uhr schrieb Paulo César Pereira de
Andrade <address@hidden>:
>
> Em dom, 25 de ago de 2019 às 04:30, Marc Nieper-Wißkirchen
> <address@hidden> escreveu:
> >
> > Dear Paulo,
>
> Hi, Sorry for the delay responding,
>
> > the first lines of <lightning.h> currently read:
> >
> > **
> > #ifndef _lightning_h
> > #define _lightning_h
> >
> > #if HAVE_CONFIG_H
> > # include "config.h"
> > #endif
> >
> > #include <unistd.h>
> > #include <stdlib.h>
> > #if HAVE_STDINT_H
> > # include <stdint.h>
> > #endif
> > #include <string.h>
> >
> > #if defined(__hpux) && defined(__hppa__)
> > # include <machine/param.h>
> > #endif
> > #if defined(__alpha__) && defined(__osf__)
> > # include <machine/endian.h>
> > #endif
> >
> > #ifdef STDC_HEADERS
> > # include <stddef.h>
> > #else
> > # if !defined(offsetof)
> > # define offsetof(type, field) ((char *)&((type *)0)->field - (char *)0)
> > # endif
> > #endif
> > **
> >
> > I would like to suggest to remedy the following three things:
> >
> > 1) The header file should not include "config.h" (and test
> > HAVE_CONFIG_H) because it would include the "config.h" (and test the
> > preprocessor flag) of the application using GNU lightning and not the
> > "config.h" and the preprocessor flag of GNU lightning.
> >
> > 2) Likewise, the test about HAVE_STDINT_H is meaningless as long as
> > the program using GNU lightning does not set the appropriate
> > preprocessor flag.
>
> Agreed. Only include/jit_private.h should check for config.h.
>
> > 3) A general header file should not (re-)define "offsetof". As far as
> > I see, it is not needed in the include file anyway.
> >
> > In other words, 3) is easily corrected by moving the definition of
> > "offsetof" into a header file that is private to the lightning
> > sources.
>
> Yes, it is trivial to move the definition to jit_private.h.
>
> > 1) and 2) can be corrected by replacing "lightning.h" by
> > "lightning.h.in" and using autoconf substitutions to generate a
> > customized "lightning.h". In the same go, one can also rewrite the
> > WORDSIZE tests as autoconf tests. This has also the more than welcome
> > side effect that "__WORDSIZE" (which should be considered private to
> > the compiler and standard library) doesn't need to be touched (and
> > redefined) by <lightning.h>.
>
> I was thinking of just adding some autoconf macros to -D__WORDSIZE=
> as appropriate, but it would not work because it needs to know the word size
> for a few macros and prototypes.
>
> Would you mind in making the patch for lightning.h.in? Otherwise I can
> write one in the next few days, and hopefully soon make a new release,
> including the new riscv port. If you do the patch, it would be better if done
> in a safe way, to not break a few test platforms I no longer have access,
> like HP-UX, ppc Darwin (this should not be too hard to get a vm), IRIX,
> and Tru64.
>
> > Thanks!
> >
> > Marc
>
> Thanks!
> Paulo