Re: [PATCH 05/10] msi: Add support for importing binary OBJECTS.

[Hib Eris]

On Tue, May 26, 2009 at 8:57 PM, Alexandre Julliard  wrote:
> Hib Eris  writes:
> > I have looked at my new code and it does not seem wrong to me, but
> > maybe it is somewhere else. Unfortunately, as I cannot reproduce this
> > failure, I have no idea how to debug it. Does this fail for other
> > people as well?
> >
> > Just a wild guess... could it have something to do with 64-bit vs.
> > 32-bit? I am running on 32-bit. Alexandre, did you run the test on 32
> > or 64 bit?
>
> 32-bit of course, nothing works on 64-bit <g>
>
> It's heap corruption, you forgot to allocate space for the terminating
> null in msi_import_stream_filename. And you should probably use strdupW
> instead.

Ahhh, I see! I will create a new patch. Thanks.

A question about using strdupW: In my patch I use msi_alloc and
msi_realloc to allocate memory for the returned string and use
msi_free to free it again.
If I would use stdupW to create a string copy instead of msi_alloc,
would it then still be okay to use msi_realloc and msi_free? According
to
http://source.winehq.org/WineAPI/StrDupW.html I should use LocalFree
to free the memory?

I've seem some more places where msi_free and StdDupW were mixed, so I
guess it will work. But is it considered good practice?

Hib

[PATCH 05/10] msi: Add support for importing binary OBJECTS.

[Michael Stefaniuc]

Hib Eris wrote:
> On Tue, May 26, 2009 at 8:57 PM, Alexandre Julliard  wrote:
> > Hib Eris  writes:
> > > I have looked at my new code and it does not seem wrong to me, but
> > > maybe it is somewhere else. Unfortunately, as I cannot reproduce this
> > > failure, I have no idea how to debug it. Does this fail for other
> > > people as well?
> > >
> > > Just a wild guess... could it have something to do with 64-bit vs.
> > > 32-bit? I am running on 32-bit. Alexandre, did you run the test on 32
> > > or 64 bit?
> >
> > 32-bit of course, nothing works on 64-bit <g>
> >
> > It's heap corruption, you forgot to allocate space for the terminating
> > null in msi_import_stream_filename. And you should probably use strdupW
> > instead.
>
> Ahhh, I see! I will create a new patch. Thanks.
>
> A question about using strdupW: In my patch I use msi_alloc and
> msi_realloc to allocate memory for the returned string and use
> msi_free to free it again.
> If I would use stdupW to create a string copy instead of msi_alloc,
> would it then still be okay to use msi_realloc and msi_free? According
> to
> http://source.winehq.org/WineAPI/StrDupW.html I should use LocalFree
> to free the memory?

StrDupW and strdupW aren't the same.
> I've seem some more places where msi_free and StdDupW were mixed, so I

msi doesn't uses StdDupW but strdupW.
> guess it will work. But is it considered good practice?

Checkout dlls/msi/msipriv.h. You'll find there the implementation of 
msi_alloc, msi_free and strdupW. Those are basically wrappers around 
HeapAlloc/HeapFree.

bye
    michael