On Thu, Aug 21, 2008 at 02:57:02PM -0600, Brad Nicholes wrote: > >>> On 8/21/2008 at 12:19 PM, in message > <[EMAIL PROTECTED]>, > <[EMAIL PROTECTED]> wrote: > > http://bugzilla.ganglia.info/cgi-bin/bugzilla/show_bug.cgi?id=200 > > > > [EMAIL PROTECTED] changed: > > > > What |Removed |Added > > ---------------------------------------------------------------------------- > > CC| |[EMAIL PROTECTED] > > AssignedTo|ganglia- |[EMAIL PROTECTED] > > |[EMAIL PROTECTED]| > > |et | > > > > > ------- Additional Comments From [EMAIL PROTECTED] 2008-08-21 11:19 ------- > > there was a reason why libgmond values were set to NULL (maybe so it can be > > written over if needed, and easily detected to be an empty value, but can't > > recall). > > > > there is a note in gmond.conf which is related : > > > > B<NOTE>: Currently gmetad cannot process data sources that do not > > have a cluster tag. This will be remedied in the future. > > Carlo,
Brad, > So what do you want to do with the patch? short version: punt for next release and tag 3.1.1 without it longer version below > I don't think that specifying a cluster default name is going to hurt > anything since having no cluster name is a misconfiguration. Also the > default gmond.conf file provides a default anyway for all of the cluster > values. true, but this patch is also changing the way this structure is allocated from heap to some data segment closer to the code and which needs to be treated more carefully (as it will be a perfect target for a security point of view) and this patch is changing libganglia and therefore, at least by definition affecting more than just gmond. > After looking at the XML parsing code, I can see why the above note was > necessary. The parser can't handle a host without a cluster. presume you mean the gmetad XML parsing code here, and yes I would expect that to be the case (a cursory look shows it is probably enforced by our DTD) but then some other consumers from gmond data might not have that requirement at all. haven't had time yet to look at the impact of the change in the rest of the code to do any review, but as I mentioned in that comment, the fact that someone decided long ago to put a comment in the configuration instead of fixing it the obvious way (which should be something like your patch), probably means that there was a reason why it wasn't a good idea (at least at that time), and then again cursory looking of the code shows that those attributes are being "free" and reassigned to point to heap allocated objects and therefore in some platforms they could fail and are of undefined behaviour. Carlo ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Ganglia-developers mailing list Ganglia-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ganglia-developers
