Re: [Exult-general] Cleanup

[Max H. <ma...@qu...>]

Am 12.12.2005 um 17:57 schrieb Jeff Freedman:

> Max Horn wrote:
>
>>
>> * Cleanup configure cruft: The following configure options seem   
>> pointless/obsolete or of only marginal value, so I'd like to  
>> remove  them:
>>   --enable-alternate-allocator
>>   --enable-storage-initialisation (we shouldn't rely on this!)
>>   --enable-storage-poisoning (many OSes / debuggers provide  
>> native  ways for this)
>>   --enable-awful-warnings (these are indeed awful, and sometimes   
>> 'wrong' / bad advice)
>>   --enable-std-namespace (GCC 2.95 specific, obsolete)
>>   --enable-repo
>>   --enable-external-templates
>>   --enable-mmx
>>   --enable-3dnow (dito)
>
> No problem with any of the above.  If I wanted to 'poison' memory  
> to find a specific bug, I'd just write an alternate allocator for  
> temporary use.  Actually, I'd use Valgrind as my first choice  
> anyway for that sort of thing.
>
Done.

>>
>> * As a consequence of the above, alloc.cc would be removed
>
> No problem.

Done.

>
>>
>> * Replace autoarray by std::vector
>
> Fine.  Although, does autoarray have any special features we rely on?

I don't think so... well, there is something, but I'd rather call it  
a bug or a misfeature... in fact, I am surprised that it's not  
causing crashes, so maybe I am missing something subtle.

The difference is that vector properly uses the assignment operator /  
copy constructor when copying element, or when resizing. Compare this  
to autoarray, which simply does a memcpy.

Now, we currently use autoarray in exactly one spot: class  
Shapes_vga_file uses it for an array of Shape_infos. And Shape_info  
has a private copy constructor and a private operator =. On purpose,  
obviously: It wouldn't be a good idea to just copy the content of the  
Shape_info byte by byte, because the destructor deletes a lot of the  
content, and if we simply copied the data bytewise, we'd free things  
that are potentially still in use...

And that's where I am surprised things are working: When a set_size  
call is made on the autoarray, then the old data is copied, using  
memcpy, to a new array; and the old one is then deleted. This delete  
operation should in theory call the destructors of all the objects we  
just memcpy'ed, which then should cause lots of weird bugs and  
crashes due to freed memory still being used...

I am not sure why this is working right now, which is why I won't  
touch it for now, until somebody can tell me what's going on :-). But  
other than that, the only two changes that should be necessary is  
replacing autoarray by std::vector in shapevga.h, and replacing calls  
to set_size() by resize().


>
>>
>>
>> * Replace Exult_vector / Exult_queue, if possible, by vector /  
>> queue,  or at least make them a little bit nicer.
>
> Exult_vector is derived from std::vector with some other methods  
> added (although I'm not sure how much those are used).

Yeah, of course it has to be carefully checked here whether this  
switch is possible / sensible.


Cheers,
Max