On Thu, 10 Sep 2009 21:20:04 +0530 Krishna Srinivas <krishna at gluster.com> wrote: > Now, failing to check for NULL pointer here is a bug which we will fix > in future releases (blame it on our laziness for not doing the check > already!) Thanks for pointing it out. Really, this was only _one_ quick example of which there are numerous in your code. Look at all CALLOC/MALLOC calls. Most of them are not safe. Look at your documentation. It is quite a mess. There seems to be no intention to document which version knows which options, they are pretty different. It would have been easy-go if you started that from the very first release and just copied all the docs to a new tree deleting dead options and adding the new ones, linking the docs to version numbers. This would allow people to find out what is really a valid option. I will not stop to post every single case that looks bogus, even without understanding a single bit of the semantics. > Talking about analogy, in a car assume that engine is the glusterfs > and tyres the kernel. If you get flat tyres and the car doesn't move > you can't blame the engine! Boy, you really entered cloud nr 9. To bring your example down to reality I'd rather suggest the kernel being the engine and and glusterfs being the rear view mirror. The car can live without, nice to have one though. > Thanks > Krishna And todays' example of coding is in glusterfs-2.0.6/transport/socket/src/name.c. # grep -n UNIX_PATH_MAX name.c 95: if (!path || strlen (path) > UNIX_PATH_MAX) { 281: if (strlen (connect_path) > UNIX_PATH_MAX) { 284: strlen (connect_path), UNIX_PATH_MAX); 321:#ifndef UNIX_PATH_MAX 322:#define UNIX_PATH_MAX 108 323:#endif 325: if (strlen (listen_path) > UNIX_PATH_MAX) { 329: strlen (listen_path), UNIX_PATH_MAX); Now what does that mean? UNIX_PATH_MAX used in lines 95,281,284 and then in 321 it comes to programmers' mind that it may be undefined? Ah well, things get more interesting: libglusterfs/src/compat.h:#define UNIX_PATH_MAX 108 libglusterfs/src/compat.h:#define UNIX_PATH_MAX 104 libglusterfs/src/compat.h:#define UNIX_PATH_MAX 104 libglusterfs/src/compat.h:#define UNIX_PATH_MAX 108 libglusterfs/src/transport.h: char identifier[UNIX_PATH_MAX]; Ok, if you define it depending on the OS, how can it be absolute 108 in socket/src/name.c (and elsewhere) ? Remember, no semantics analyzed, just reading ... may as well be bs from me. -- Regards, Stephan