re: JanitorialProjects/ReplaceMalloc: remove malloc and free from dlls folder

[Dan Kegel]

Yegor wrote:
> it's my first contribution to wine...

Welcome!
> so I wanted to start with some simple task.

Changing memory allocation might not be simple.

Your patch touches two different DLLs.
You should probably split this patch up, one patch per dll.

Also, the fact that your change to d3dcompiler_43 only
changes a malloc and not a corresponding free is
interesting.  Is there a leak?  Better run this past the
author of that code.
> The source code could be compiled without problems after applying the patch.

Verifying that it compiles isn't enough.  You also have to
verify by eye that this change doesn't introduce any alloc/free mismatches,
and check that it doesn't introduce any conformance test regressions.
Since not all tests pass, you need to run the tests first without
your change, and then with your change, and understand any
differences in the results.
Since you're messin' with malloc, you should also do that
with WINEDEBUG=+heap.
- Dan

JanitorialProjects/ReplaceMalloc: remove malloc and free from dlls folder

[Matteo Bruni]

2010/12/30 Dan Kegel <dank(a)kegel.com>:
> Yegor wrote:
> > it's my first contribution to wine...
>
> Welcome!
> > so I wanted to start with some simple task.
>
> Changing memory allocation might not be simple.
>
> Your patch touches two different DLLs.
> You should probably split this patch up, one patch per dll.
>
> Also, the fact that your change to d3dcompiler_43 only
> changes a malloc and not a corresponding free is
> interesting.  Is there a leak?

That string is freed by wpp, which is a native library thus it uses
free(). So the code is correct as it is now, with the malloc(). There
were already a couple of times people came up with similar patches, I
can't find them right now though.

Maybe it's just time to remove the "replace malloc with HeapAlloc"
task from the wiki? Or, at least, add a note to explain that we expect
the current Wine code to be "compliant".

JanitorialProjects/ReplaceMalloc: remove malloc and free from dlls folder

[Dan Kegel]

On Fri, Dec 31, 2010 at 3:48 PM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
> Maybe it's just time to remove the "replace malloc with HeapAlloc"
> task from the wiki? Or, at least, add a note to explain that we expect
> the current Wine code to be "compliant".

Done.