Re: r21936 is damn suspicious

On 07.09.2011 18:03, Immanuel Scholz wrote:
> Hi,
>
> Can I join the discussion?
>
> On 06.09.2011 19:04, Stefan Küng wrote:
>>>>> May I wonder why r21936 was committed?
>>>> To reduce the memory allocated on the stack by allocating the
>>>> memory on the heap instead.
>>> I wonder if that reduction is for the sake of reduction or stack
>>> allocation was causing observable problem.
>>
>> Better be safe than sorry :)
>
> That's a very unusual point of view, as I usually only see changes from
> "new[] / delete[]" toward stack allocations labeled with "better safe
> than sorry".
>
> I would always recommend using the stack instead of the heap if its
> about small KB-sizes like in your commit and the function is not in
> heavy recursive functions.
>
> Heck, I even recommend using the non-standard _alloca() instead of
> new[]/delete[] to allocate short-term memory for cases, when the size is
> not known at compile time!

Apparently, you've never had to write code for a microcontroller where
you're lucky if you have 1kb of stack available :)

Sure, if you know exactly how much stack you have available, then you
can use it even for bigger memory chunks.
But many of our functions and classes/methods are also used in the shell
extension part. And there we don't have any control over stack size: an
app that already uses up the bigger part of its stack and then opens a
file-open/save dialog will load our shell extension: if we then use a
bigger chunk of memory on the stack, we risk an overflow...

> Another thing to consider: Allocations on the heap can require thread
> locks, global memory table updates, cache flushes and cause memory
> fragmentation. Stack allocations decrement a register.
>

Sure, I consider that every time I know a function is called multiple
times and is performance relevant. But in these situations, they're not.

>>>>> Currently it uses raw pointers and this is not exception-safe.
>>>>> Also
>>>> I don't quite see the problem here?
>>> Well, if an exception is thrown between "new" and "delete" the latter
>>> in not invoked and the object is leaked. That's why we have auto_ptr,
>>> auto_buffer and a gazillion of other classes.
>>
>> Sure, but using auto_ptr or something similar only makes sense if we
>> would actually catch the exception - but we don't. Instead, an exception
>> would trigger the crash report dialog so the memory leak is not a problem.
>
> As Dmitry pointed out: memory leak != resource leak.
>
> Also, to add to Dmitry's problem list, using unprotected delete[]'s make
> the code fragile against early return statements and its just a bad
> example for other code where it may not be that easy. Even the "delete
> vs delete[]" issue in C++ alone makes me not to want to use new[]
> whenever I can.
>
>
> So to be "better safe than sorry", I would recommend using the stack
> allocations here too. ;-)

I'd rather use some auto pointer instead.
Care for a patch?
(I'm still catching up on all the emails - was stuck in traffic for two
hours!)

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2836522
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].