On 2012-10-09 00:51, Christophe wrote:
Hi,

If you'd allow me, I'd like to say this about this: (take it for what
its worth)

----- Rodolfo García Peñas <[email protected]> a écrit :
On Sat, 06 Oct 2012, Carlos R. Mafra escribió:

> On Sat,  6 Oct 2012 at 12:22:26 +0200, Rodolfo García Peñas wrote:
> >
> > Some functions malloc an free are used.
> > These functions should be replaced (always?) by wmalloc and wfree.
>
> Why do you think they should be replaced?

Hi Carlos,

This is the function wfree:

[...]

It checks if the pointer exists, then free it and set it to NULL. IMO is better to avoid free a NULL pointer and get a crash with double free memory error.

About wmalloc:

[...]

Is better because:

1. If memory is not available, sleep and retry -> No crash.

Unfortunately, it retries only once, and after a too short time to
realistically expect some memory to have been freed, so it is likely
that it will "crash" anyway.


2. If no memory is available at all, then do a "order" exit -> No crash.

There are many discussion that highlight that this is a bad behaviour
at this place, because the wmalloc is actually in a library, and this
exit actually lets no other option to the caller program - there are
some cases when we can simply cancel the current action and continue
instead of crashing.


3. Set the memory to "0". It is interesting for different compilers (sometimes set the memory to 0, sometimes no). -> Memory always set to 0, less errors, no more memory initialization.

That is a point I like about this wmalloc, however this can be a
performance issue when allocating big memory chunks (for example:
loading the background image, when you obviously know that the buffer
will be filled with image's data)


4. You can do "wwin = wmalloc(sizeof(wwin))" instead "wwin = (struct wwin*) malloc(wwin)"

Unfortunately this is not true, because both return the same type: void*
So, in both case you should:
 - either you use explicit type cast, because you're in C++ which is
strict about it;
 - or you don't, because in plain C that is what void* was made for,
a point-to-anything type.



Is my opinion.

And it looks like from the previous text that my opinion is not, but
that is not true. I think you're quite right about using wmalloc/wfree
everywhere:

1. Consistency. Because it is always better to have a code that do
things in many different ways when one was explicitly setup for this;

2. Security, because it is always a bad idea to mix different memory
allocator functions together (try to 'delete' something that was
'wmalloc'ed in C++ and you'll get my point);

3. Ease-of-use, because the day someone will want to make some
analysis on memory usage, he'll be happy that we had used the same
function everywhere;

4. One-place-to-fix-all, because if someday we want to make some
changes to the wait-and-retry or to the exit-on-memfull behaviour,
we'll be happy that it is found in one single place.


Yes, you did a good analysis. If you think that the current wmalloc/wfree can be better (for example, your point about memory available retry), we can change it.

kix
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/


--
To unsubscribe, send mail to [email protected].

Reply via email to