Re: msvcp90: Add codecvt ctors and dtors

[Piotr Caban]

Hi,

On 04/16/12 02:50, William Panlener wrote:
> typedef struct {
> +   locale_facet facet;
> +   /* FIXME: type definition in standard but var here */
> +   enum { ok, partial, error, noconv } result;
> +} codecvt_base;
> This structure has incorrect size. It should not contain result variable.
> +typedef struct {
> +   codecvt_base base;
> +   /* FIXME: incomplete struct definition */
> +} codecvt_char_char_mbstate;
> It's better to define whole structure. Also other structures are named 
> differently, it would be probably better to name it codecvt_char.
> +typedef struct {
> +   codecvt_base base;
> +   /* FIXME: incomplete struct definition */
> +} codecvt_short_char_mbstate;
> +
> +typedef struct {
> +   codecvt_base base;
> +   /* FIXME: incomplete struct definition */
> +} codecvt_wchar_char_mbstate;
> There's no reason to define separate structures for unsigned short and 
> wchar_t variants of the class. Probably the only difference between this 
> classes is typeinfo structure.
> +/* FIXME: Potentially unused */
> +/* ?id@?$codecvt(a)DDH@std@@2V0locale(a)2@A */
> +locale_id codecvt_char_char_mbstate_id = {0};
> I don't understand this comment. The id fields will be used by locale class.
> +/* ??1codecvt_base(a)std@@UAE(a)XZ */
> +/* ??1codecvt_base(a)std@@UEAA(a)XZ */
> +DEFINE_THISCALL_WRAPPER(codecvt_base_dtor, 4)
> +void __thiscall codecvt_base_dtor(codecvt_base *this)
> +{
> +    FIXME("(%p)\n stub!", this);
> +    TRACE("(%p)\n", this);
> +}
> There's no need to use both FIXME and TRACE.
> +/* ??1?$codecvt(a)DDH@std@@MAE(a)XZ */
> +/* ??1?$codecvt(a)DDH@std@@MEAA(a)XZ */
> +DEFINE_THISCALL_WRAPPER(codecvt_char_char_mbstate_dtor, 4)
> +void __thiscall codecvt_char_char_mbstate_dtor(codecvt_char_char_mbstate *this)
> +{
> +    /*FIXME stub*/
> +    codecvt_base_dtor(&this->base);
> +    TRACE("(%p)\n", this);
> +}
> It's better to print a fixme message here.
> +DEFINE_RTTI_DATA(codecvt_char_char_mbstate, 0, 1,&locale_facet_rtti_base_descriptor, NULL, NULL, " ?AV?$codecvt(a)DDH@std@@");
> +DEFINE_RTTI_DATA(codecvt_short_char_mbstate, 0, 1,&locale_facet_rtti_base_descriptor, NULL, NULL, " ?AV?$codecvt(a)GDH@std@@");
> +DEFINE_RTTI_DATA(codecvt_wchar_char_mbstate, 0, 1,&locale_facet_rtti_base_descriptor, NULL, NULL, ".?AV?$codecvt(a)_WDH@std@@");
> The RTTI data for these structures is incorrect.
> +    __ASM_VTABLE(codecvt_base, ""/*FIXME*/);
> +    __ASM_VTABLE(codecvt_char_char_mbstate, ""/*FIXME*/);
> +    __ASM_VTABLE(codecvt_short_char_mbstate, ""/*FIXME*/);
> +    __ASM_VTABLE(codecvt_wchar_char_mbstate, ""/*FIXME*/);
> It's better to create correct virtual functions table. This may lead to 
> some strange crashes.