Hi Matt, On Friday 15 December 2006 19:06, matt massie wrote: > [...] i just went and read your bug in bugzilla and see no problem with it > being put into gmetad and frontend.[...]
Thanks. > my only concern about your patch is that people using filesystems that > can take a "/" in a filename will find that their data is written to > differently named files. Yes, I agree (although "/" is never allowed under Linux VFS). There's also the reverse case: some OSes have more strict constraints on filenames (Windows, I believe, is one). I think the only way to handle this would be some static definitions separated by #if .. #elif ... etc. It would be nicer to use autoconf for this (automatically detect whether potentially dangerous characters are bad), but this would fail whenever cross-compiling (i.e. host != target). The other limitation of the patch is that it assumes each character is independent of the previous or subsequent ones: that the character's position doesn't matter. With Windows in particular, the rules for what what is a valid filename are very complicated and sometimes position dependent. I don't believe it is fully understood (see [1] and "ntdll:path" results in [2]). [1] http://cvs.winehq.org/cvsweb/~checkout~/wine/dlls/ntdll/tests/path.c [2] http://test.winehq.org/data/200612161000/ Trying to capture this behaviour for Windows builds might be sufficiently difficult that simply escape all potentially troublesome characters ( '$', '\\', etc...) would be easier. This would lead to potential over-escaping (escaping paths unnecessarily), but perhaps this is inevitable. > if the developers here are ok with that, i'm happy to just drop in the > patch. Despite the above problems, I'm happy for the patch to go in. The algorithm can be refined for other platforms, but I don't have ready access to the other platforms. I hope someone else could take this on. > we also need to a add a simple check to make sure free() isn't passed NULL > (some operating systems don't like that). Err, OK. By all means, we can add this test. As an aside, Matt, do you know which specific operating systems don't like free(NULL)? This topic has come up on some other mailing lists and (at that time) no one could name a specific operating systems libc (or equiv) that crashes on free(NULL). I believe both C99 and C89 define free(NULL) as a noop. From some (limited) investigation, I've found three reported cases of free(NULL) causing a crash: Win32 (NT, ~1997) [3] Win32 (unknown, ~2000) [4] and SunOS (unknown, ~2000) [5] [3] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4080278 [4] http://www.cygwin.com/ml/gdb-patches/2000-04/msg00244.html [5] http://www.sourceware.org/ml/gdb-patches/2000-04/msg00232.html Both Microsoft [6] and Sun [7] claim that free(NULL) is a no-op. This may now be true. [6] http://msdn2.microsoft.com/en-us/library/we1whae7(VS.80).aspx [7]http://docsun.cites.uiuc.edu/sun_docs/C/solaris_9/SUNWaman/hman3c/malloc.3c.html Does anyone know if free(NULL) is still a problem with these platforms? How big is the install-base of these "broken" operating systems? Are there any other platforms that have problems with free(NULL) that I've missed? [...] > the ganglia team is very informal. we don't really have a formal code > process which is both good and bad. sorry for not being as responsive > as we have in the past. No! Not a problem at all. I quite like informal teams. As a suggestion, perhaps this could be documented somewhere (perhaps as a FAQ "how do I contribute code?"). Any such documentation doesn't have to be too elaborate, just enough to let people know what to do. Cheers, Paul.
pgpmYK9m4uvnL.pgp
Description: PGP signature
