Re[2]: [Gc] Dependency tracking for configuration macros

Ivan Maidanski ivmai at mail.ru
Thu May 21 04:25:51 PDT 2009


Hi!

Petter Urkedal <urkedal at nbi.dk> wrote:
> On 2009-05-20, Ivan Maidanski wrote:
> > Petter Urkedal <urkedal at nbi.dk> wrote:
> > > The attached patch adds AC_CONFIG_HEADER to configure.ac, so that the
> > > command-line "-D"-options are replace by the conditional inclusion of a
> > > generated config.h header.  The major part of the patch is the addition
> > > of documentation to configuration macros.
> > 
> > Opinion:
> > 1. instead of inclusion of "gc_config_macros.h" from misc .c files (like backgraph.c one), I'd rather include "gc.h" - see my patch in http://permalink.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/2575 (mostly taken from GCC/boehm-gc).
> 
> Yes, I don't mind that.  If that patch is to be committed, I could wait
> and rebase my changes.  (Hans?)
> 
> > 2. Inclusion of a "private/..." file from a public header ("gc_config_macros.h") is not a good idea (even if it is ifdef'ed), I think.
> 
> I principal I agree, but the problem is that "gc.h" includes
> "gc_version.h", and the latter file must be included after the Autoconf
> macros, if they are to be included.  This is the reason I felt it
> necessary to add GC_PRIVATE_CONFIG_H as a supplement to the standard
> CONFIG_H definition.  I consider the ifdef-guard safe since
> GC_PRIVATE_CONFIG_H is in the "namespace" of the collector, but granted,
> it looks a bit ugly to an end-user who reads the include file.

Another way:
1. include "private/gc_priv.h" at the beginning of backgraph.c, checksums.c, gcj_mlc.c, real_malloc.c and gc_pmark.h;
2. include "gc_version.h" and configuration file (ifdef'ed) at the beginning of "gc_priv.h" (before including "gc.h").
3. I'd like also to see #define GC_BUILD (ifndef'ed) in gc_priv.h after config file included.

What do You think?

> 
> On a related note, it might also be of interest to make configuration
> macros available from public headers.  In my own code I usually
> transform the config.h by adding a project-specific prefix to all macros
> which do not already start with said prefix.

May be. But how many macros could be there? GC_THREADS, GC_DLL (Win32 only), GC_USE_LD_WRAP (Unix only), the rest are of rare use.

The other Q: are You going to make this 'public' config auto-gen'ed by GC too? Otherwise, it's not clear how this would be looking in the client code:

#include "my_app_config.h"
#define I_HAVE_CUSTOM_GC_API_CONFIG /* looks a bit ugly here */
#include "gc.h"

If we move "define I_HAVE_..."  to my_app_config then the Q: isn't that easy to just have "include my_gc_config.h" in it ("my_app_config.h") instead (or directly have "define GC_THREADS"... unless you have a standalone cfg script for the gc API in your app)?

> 
> > > I tested the patch on various configurations on Linux x86_64 and i386 by
> > > diff-ing the disassembly of the object files before and after applying
> > > the patch.
> > > 
> > > -----%<-----
> > > Dependency tracking for configuration macros.
> > > 
> > > configure.ac, acinclude.m4:
> > >         * Invoke AC_CONFIG_HEADER.
> > >         * Added documentation of configuration macros.
> > >         * Commented out the unused macros STACKBASE and DATASTART_IS_ETEXT.
> > 
> > STACKBASE should be also removed in misc.c file.
> 
> Yes, I was puzzled by that lone #undef...

Bye.


More information about the Gc mailing list