Re: [Lightning] config.h should not be included in the library header

[Paulo César Pereira de Andrade]

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