[Bf-committers] Code cleanup woes

[Campbell Barton]

Of course this is not a "fix", but as our API's change, it makes sense
(sometimes) point to use the improved API's in existing code.


It came to my attention that quite a bit of our code was doing
(calloc/memcpy/free) just to resize an existing array.

With re-calloc you can do...
---
temp = (AviIndexEntry *) MEM_recallocN(movie->entries, (frame_num + 1)
* entry_size);
---

Compared to old code.
---
	temp = (AviIndexEntry *) MEM_mallocN ((frame_num+1) *
	    (movie->header->Streams+1) * sizeof(AviIndexEntry),"newidxentry");
	if (movie->entries != NULL) {
		memcpy (temp, movie->entries, movie->index_entries *
(movie->header->Streams+1)
		    * sizeof(AviIndexEntry));
		MEM_freeN (movie->entries);
	}
---
When looking into a bug report in areas of code like this you may want
to double check the sizeof(), offsets, alloc-size etc is correct (off
by one cause uninitialized memory of writing out of memory bounds), so
even if its working its more code to wade through and check from time
to time.

Not to say going over and replacing all code is something we should
do, but with an API that makes some common expression more concise - I
believe it can be good to do so with care (and testing of course,
which I did, just not adding from zero).
Devs use existing code as a reference, so if we use our API's in a
clear way I think it helps us in the long term with GSOC, patches -
new modules.

IMHO we should try to have our own code be an example of what we would
like others to develop in blender.

In practice its a compromise, some refactors/cleanups are just too
disruptive, error-prone or time consuming.

But I disagree with Ton's comment: "New code should be totally and
undeniably better, for users. Not for coders."

Coders have to read/debug blenders existing code a _lot_. - Reducing
confusion is important and reduces errors we make when we do end up
making functional changes to the code (really! - quite a few bugs in
the tracker I found have been caused by devs who improve blender but
miss checking for non obvious side-effects or just having trouble with
confusingly written code).

In the case of our particle code, I agree that we can just leave it
and not try do more cleanup - just maintain until its re-written.

On Sun, Jan 27, 2013 at 8:29 PM, Ton Roosendaal <ton at blender.org> wrote:
> Hi,
>
> You miss the point Jason. It's not about being stupid or not thinking enough.
> It shouldn't even have come up in a coders mind to "fix" it.
>
> -Ton-
>
> ------------------------------------------------------------------------
> Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
> Blender Institute   Entrepotdok 57A  1018AD Amsterdam   The Netherlands
>
> On 26 Jan, 2013, at 22:02, Jason Wilkins wrote:
>
>> I think this would not have broken if MEM_reallocN was exactly like
>> the standard library realloc.  The slight differences probably mislead
>> whoever was making this change.
>>
>> On Sat, Jan 26, 2013 at 6:42 AM, Ton Roosendaal <ton at blender.org> wrote:
>>> Hi all,
>>>
>>> Here's another nice illustration of code cleanup that caused bugs;
>>> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=54058
>>>
>>> The idea of "replace calloc + memcpy with recalloc" sounds nice, but it breaks the case when memcpy is of length zero. (zero hairs, use 'add' brush in particle mode).
>>>
>>> May I quote Joelonsoftware here?
>>>
>>> "Old code has been used. It has been tested. Lots of bugs have been found, and they've been fixed. There's nothing wrong with it. It doesn't acquire bugs just by sitting around on your hard drive. Au contraire, baby!"
>>>
>>> Great fun read:
>>> http://www.joelonsoftware.com/articles/fog0000000069.html
>>>
>>> Of course we should never be afraid to recode parts of Blender, but it shouldn't be with the naive assumption that the old bad messy code wasn't functional. New code should be totally and undeniably better, for users. Not for coders. :)
>>>
>>> We can moan and complain about our current bad particle code a lot, but it has offered a lot of good for Blender for many years. The challenge is not to make a new system with clean design, but to make a new system that's far superior.
>>>
>>> -Ton-
>>>
>>> ------------------------------------------------------------------------
>>> Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
>>> Blender Institute   Entrepotdok 57A  1018AD Amsterdam   The Netherlands
>>>
>>> _______________________________________________
>>> Bf-committers mailing list
>>> Bf-committers at blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>> _______________________________________________
>> Bf-committers mailing list
>> Bf-committers at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers



-- 
- Campbell