Re: Bug in garray.c

[Sebastian Wilhelmi <wilhelmi ira uka de>]

Hi Soeren,

> The real question is: what is zero-terminated and cleared supposed to
> mean?
> 
> Currently, zero-terminated means something like
> 
>   The array will be zero-terminated if it is not empty and its size is
>   not made smaller with g_array_set_size.
> 
> This is clearly not appropriate. Your patch will make zero-terminated
> mean
> 
>   The array is zero-terminated if it is not empty.
> 
> In my opinion, zero-terminated should either be depreceated or mean
> 
>   Thea array will be zero-terminated at all times, even when it is
>   empty.

You're of course right here. That is still wrong in my patch. I'll do it your
way.
 
> Zero-termination makes sense only in constructions like
> 
> SomeType *p
> 
> for (p = (SomeType *)array->data; *p; p++)
>   {
>     do_something (p);
>   }
> 
> Note that ANSI C does not guarantee that the above works if SomeType
> is a pointer and the zero-termination is done with memset
> (array->data, somewhere, 0). A NULL pointer does not have to be the
> address 0x0.

While that is true, I would say, that GLib wouldn't work on platforms like
these, and I really don't think there is any platform out there of any
importance, that really makes NULL-pointers non-zero (That would be quite hard
actually, as "long a; /* */ void* b = a;" would have to test a for '0' and
assign the right value to b then). So I wouldn't loose sleep about that,
knowing, that of course the standard allows it.
 
> Thus, zero-termination makes sense only for the various int types.
> 
> Currently, cleared means something like
> 
> Every time space for the array is allocated, it will be cleared to zero.
> 
> I simply don't see the point. Possibly, for debugging purposes, it may
> make sense to have cleared mean:
> 
>   Allocated space that is not part of the array is zero

Doesn't seem to be necessary.
 
> I think the bug is in the definition of the data type. This should be
> fixed before the implementation.

The definition clearly (in my view) is:

All unassigned members of the array (as created by g_array_set_size) are zero.

So actually the following theorem holds:

Theorem: 
 {x|x is a zero_terminated array} is a real subset of {x|x is a cleared array}
 
;-)

Ok, I admit, one could as well argue, that after g_array_set_size only the
data[length] member has to be zero, thus making the two properties more
orthogonal. I think, I'll do just that.

> > The same applies to GPtrArray, which is not supposed to be zero_terminated
> > but should be cleared. So I removed the final
> >
> >    array->pdata[array->len - 1] = NULL;
> 
> NULL terminated (as opposed to a 0x0 terminated/cleared) arrays could
> help catch off-by-one bugs.

I have another version here (see my post on memprof friendlyness), that zeros
out all memory, that is kept by GLib, but not used anymore, as memprof (or a
garbage collector) would consider memory still used, that is referenced from
such unused places. This doesn't seem like a good idea in general however.

Bye,
Sebastian
-- 
Sebastian Wilhelmi                   |            här ovanför alla molnen
mailto:wilhelmi@ira.uka.de           |     är himmlen så förunderligt blå
http://goethe.ira.uka.de/~wilhelmi   |