oleaut32: fix regression at MSFT typelib parsing

Finally, I managed to figure out the proper fix for the crash on regsvr32
msvbvm60.dll (and probably some other DLLs too). It turns out that the
field from which TLBFuncDesc->Entry is filled is supposed to indicate the
function entry point that implements the function described in the
typelib. The roper interpretation of the entry is right there as a
comment:

BSTR Entry;            /* if its Hiword==0, it numeric; -1 is not
present*/

What this means is that if TLBFuncDesc->Entry has a high word of 0, it is
supposed to be a DLL function ordinal. Otherwise, it is an offset of the
function name (for lookup-by-name in the DLL). When this field was
directly copied from the binary block (and the block kept around, with a
memory leak), everything worked as expected. However, the patch to
allocate space for the function name in the Entry-as-funcname case broke
the Entry-as-ordinal case, since the ordinal value is not a proper
pointer, and therefore it is incorrect to use it as such (like calling
SysAllocString on it). This patch fixes the regression by copying the
ordinal value when indicated by the FKCCIC flag, and checking the HIWORD
before trying to free the allocated string in the Entry-as-funcname case.

BTW, could any of you please comment on the previous patch for removing
the use of CreateCompatibleDC in the implementation of GetDIBits() ? I
sent it a few days ago, but it seems to have been ignored (no commit, no
critique on why it is being rejected). Otherwise I will send it again in
the assumption that it slipped through the cracks somewhere.

Changelog:
* Fix regression on MSFT typelib parsing of function records by allocating
  a string copy only when indicated by FKCCIC flag, and preserving the
meaning of value as function ordinal otherwise.

Alex Villacís Lasso

[Detlef Riekenberg]

On Fr, 2006-11-03 at 11:42 -0500, [e-mail] wrote:
> Finally, I managed to figure out the proper fix for the crash on
> regsvr32 msvbvm60.dll

Great!
Thanks for your investigation.

I have no Idea about ole, but...
> MESSAGE("\thelpstring: %s\n", debugstr_w(pfd->HelpString));
> - MESSAGE("\tentry: %s\n", debugstr_w(pfd->Entry));
> - if (HIWORD(pfd->Entry) == 0)
> -
>  MESSAGE("\tentry (ordinal): 0x%04x\n", (INT)pfd->Entry);
> - else if (pfd->Entry != (void *)-1)
> -
>  MESSAGE("\tentry (string): %s\n", debugstr_w(pfd->Entry));
> - else
> -
>  MESSAGE("\tentry (invalid): -1\n");

Pointers with "HIWORD() == 0" are already handled by debugstr_w().
You will get a "#" followed by the number.

I suggest to reuse the old code:
MESSAGE("\tentry: %s\n", (entry == -1) ? "-1 (invalid)" :
debugstr_w(pfd->Entry));

Another Idea is to check only for "-1" and use 
the previous code unmodified for all other cases.

> On Fr, 2006-11-03 at 11:42 -0500, [e-mail] wrote:
> > Finally, I managed to figure out the proper fix for the crash on
> > regsvr32 msvbvm60.dll
>
> Great!
> Thanks for your investigation.
>
> I have no Idea about ole, but...
> > MESSAGE("\thelpstring: %s\n", debugstr_w(pfd->HelpString));
> > - MESSAGE("\tentry: %s\n", debugstr_w(pfd->Entry));
> > - if (HIWORD(pfd->Entry) == 0)
> > -
> >  MESSAGE("\tentry (ordinal): 0x%04x\n", (INT)pfd->Entry);
> > - else if (pfd->Entry != (void *)-1)
> > -
> >  MESSAGE("\tentry (string): %s\n", debugstr_w(pfd->Entry));
> > - else
> > -
> >  MESSAGE("\tentry (invalid): -1\n");
>
> Pointers with "HIWORD() == 0" are already handled by debugstr_w().
> You will get a "#" followed by the number.
>
> I suggest to reuse the old code:
> MESSAGE("\tentry: %s\n", (entry == -1) ? "-1 (invalid)" :
> debugstr_w(pfd->Entry));
>
> Another Idea is to check only for "-1" and use
> the previous code unmodified for all other cases.
>
> --
>
> By by ... Detlef

New version of the patch, with suggested changes.

Changelog:
* Fix regression on MSFT typelib parsing of function records by allocating
  a string copy only when indicated by FKCCIC flag, and preserving the
meaning of value as function ordinal otherwise.

Alex Villacís Lasso