On Wed, Apr 02, 2008 at 04:31:37PM -0600, Brad Nicholes wrote: > >>> On 4/2/2008 at 2:43 PM, in message <[EMAIL PROTECTED]>, Carlo > Marcelo Arenas Belon <[EMAIL PROTECTED]> wrote: > > On Wed, Apr 02, 2008 at 10:08:04AM -0600, Brad Nicholes wrote: > >> >>> On 4/1/2008 at 10:28 PM, in message <[EMAIL PROTECTED]>, Carlo > >> Marcelo Arenas Belon <[EMAIL PROTECTED]> wrote: > >> > > >> > ok, but then if the application also has a need to access the metric data > >> > (like gmond) then we will end up including gm_protocol.h twice, and that > >> > will lead to redefined symbols (that headers is generated by rpcgen and > >> > might not have protection against multiple inclusion) > >> > >> No, rpcgen does the right thing in the header to make sure that it doesn't > > get included multiple time. > >> > >> #ifndef _GM_PROCOTOL_H_RPCGEN > >> #define _GM_PROCOTOL_H_RPCGEN > >> > >> ... > >> > >> #endif > > > > that is implementation dependent and not required (RFC1832), so not all > > implementations of rpcgen do that (indeed it might be assumed to be > > discouraged as rpcgen is meant to generate plain C code and therefore > > implicitly not require a preprocessor) > > > > as I explained before, cygwin's at least doesn't and will therefore result > > in a broken build. > > > > if you want to rely in this implementation specific behaviour then the > > gm_protocol.x file should be processed and the result committed instead > > of relying on rpcgen to do that at compile time, but will be IMHO much > > simpler > > just to do the header change I suggested and avoid committing generated > > files > > which will then result in maintenance headaches later when they get modified > > without changing the source first and regardless of the warnings about > > not doing that which are to be found on its header. > > It sounds like if the cygwin version of rpcgen is not producing the > appropriate #ifndef, then there is a high potential for breaking the cygwin > build at anytime anyway. All that needs to happen is the unintentional > include of gm_protocol.h in any of the source files and things would continue > to work correctly on all other platforms except cygwin.
cygwin is only relevant here as an example of a platform where the assumption that the generated header from rpcgen will have include guards is broken and results in a broken build. as I explained before there is nothing in the specification for rpcgen which can be used to validate that include guards will always be generated in all supported platforms and so the build is broken for all the platforms that do not, and from that list, cygwin is only an example. if we expect ganglia to be as portable as it is scalable (as preached in our documentation) then we can't just ignore this problem but have to commit to a solution that avoids this portability problem and I already proposed 2 : 1) add ganglia.h to gm_metric.h 2) don't generate gm_protocol.h at compile time choose your poison. > In reality there should be no reason for an application to ever include > gm_metric.h in addition to ganglia.h anyway. gm_metric.h only contains > definitions that are specific to a metric module. There is nothing in > gm_metric.h that an application would require for any purpose. the first lines for gmond/gmond.c seem to contradict that (all other mistakes already fixed in r1193 as shown before for mod_example.c) 484 massie #ifdef HAVE_CONFIG_H 484 massie #include "config.h" 484 massie #endif 1146 bnicholes #include <ganglia.h> /* for the libgmond messaging */ 1146 bnicholes #include <gm_metric.h> 1146 bnicholes and are obviously resulting in a broken build as reported Carlo ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace _______________________________________________ Ganglia-developers mailing list Ganglia-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ganglia-developers
