[Gluster-users] The continuing story ...

[Stephan von Krawczynski]

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