Dino Morelli (via RT) wrote:


Added new allocation code for resizablebooleanarray. Added push_integer,
pop_integer, shift_integer, unshift_integer.

Some other things like freeze, thaw and clone added as well because of
this.

I adapted all of the tests from intlist.t to resizablebooleanarray.t


Removed push_integer from fixedbooleanarray

Great, thanks. Some nitpicking first:

+        if(size == (PMC_int_val(SELF) - PMC_int_val2(SELF))) return;

Please read pdd07_codingstd.pod: "if (..." with a space and "return" on a new line, and ...

         if ( ! PMC_data(SELF) ) {

no blanks inside "if" for example.

The 'internal_exception's should be real_exception - some arrays are already throwing real_exception and we should eventually get rid of most of the internals.

+       pmc_arr= new ResizableBooleanArray

The variable name 'pmc_arr' isn't really appropriate.

And finally: the "unshift and shift" test (#12) is segfaulting. Looking at the code it seems that you got the allocation code wrong. Compare e.g. clone or set_integer_native with fixedbooleanarray please. You are allocating 8 times too much, because you are taking the *bit* sizes as your allocation unit. The size calculation in your clone is still more hmmm strange.

Maybe you should first add some comments and a graphic about allocation and memory layout, which also mentions MIN_ALLOC and the usage of int_val{,2}, so that it's more obvious, why and how much memory is allocated.

And please provide *one* patch, not three.

leo

Reply via email to