Re: Janitorial W->A cleanup objects/metafile.c dlls/gdi/mfdrv/init.c

[Dmitry Timoshkov]

"Tony Lambregts"  wrote:
> This one was required more that a mop and pail. In the end what I had to do is 
> convert MF_CreateMetaHeaderDisk to unicode.
>
> One burning question I have at this point is the relative merits of using 
> RtlCreateUnicodeStringFromAsciiz vs MultiByteToWideChar

MultiByteToWideChar uses Rtl* functions internally and using ntdll APIs
directly should give a slightly better performance.

RtlCreateUnicodeStringFromAsciiz also is supposed to hide the internals
of the conversion and help to avoid bugs and typos.
> @@ -62,7 +65,7 @@
>  {
>      DWORD dw1, dw2, dw3;
>      WORD w4;
> - CHAR filename[0x100];
> - WCHAR filename[MAX_PATHNAME_LEN];

Please do not increase the buffer size if there is no real need for it.
MAX_PATHNAME_LEN = 1024. It's better though to replace 0x100 by MAX_PATH IMO.
> } METAHEADERDISK;
>  #include "poppack.h"
>
> @@ -350,9 +353,9 @@
>          ERR("Not a disk based metafile\n");
>   return NULL;
>      }
> - mhd = (METAHEADERDISK *)((char *)mh + sizeof(METAHEADER));
> - mhd = (METAHEADERDISK *)((WCHAR *)mh + sizeof(METAHEADER));

Wrong.
> @@ -377,8 +380,8 @@
>         sizeof(METAHEADER) + sizeof(METAHEADERDISK));
>      mh->mtType = METAFILE_DISK;
>      size = HeapSize( GetProcessHeap(), 0, mh );
> - mhd = (METAHEADERDISK *)((char *)mh + sizeof(METAHEADER));
> - strcpy(mhd->filename, filename);
> - mhd = (METAHEADERDISK *)((WCHAR *)mh + sizeof(METAHEADER));

Wrong again.

Janitorial W->A cleanup objects/metafile.c dlls/gdi/mfdrv/init.c

[Tony Lambregts]

Dmitry Timoshkov wrote:
> "Tony Lambregts"  wrote:
> > This one was required more that a mop and pail. In the end what I had to do is 
> > convert MF_CreateMetaHeaderDisk to unicode.
> >
> > One burning question I have at this point is the relative merits of using 
> > RtlCreateUnicodeStringFromAsciiz vs MultiByteToWideChar
>
> MultiByteToWideChar uses Rtl* functions internally and using ntdll APIs
> directly should give a slightly better performance.
>
> RtlCreateUnicodeStringFromAsciiz also is supposed to hide the internals
> of the conversion and help to avoid bugs and typos.

So the trade off is speed vs potential errors. hmm...
> > @@ -62,7 +65,7 @@
> > {
> >     DWORD dw1, dw2, dw3;
> >     WORD w4;
> > - CHAR filename[0x100];
> > - WCHAR filename[MAX_PATHNAME_LEN];
>
> Please do not increase the buffer size if there is no real need for it.
> MAX_PATHNAME_LEN = 1024. It's better though to replace 0x100 by MAX_PATH IMO.

Yes that is what I intended.
> > } METAHEADERDISK;
> > #include "poppack.h"
> >
> > @@ -350,9 +353,9 @@
> >         ERR("Not a disk based metafile\n");
> >  return NULL;
> >     }
> > - mhd = (METAHEADERDISK *)((char *)mh + sizeof(METAHEADER));
> > - mhd = (METAHEADERDISK *)((WCHAR *)mh + sizeof(METAHEADER));
>
> Wrong.

OK fixed ... Thank you

Janitorial W->A cleanup objects/metafile.c dlls/gdi/mfdrv/init.c

[Dmitry Timoshkov]

"Tony Lambregts"  wrote:
> > MultiByteToWideChar uses Rtl* functions internally and using ntdll APIs
> > directly should give a slightly better performance.
> >
> > RtlCreateUnicodeStringFromAsciiz also is supposed to hide the internals
> > of the conversion and help to avoid bugs and typos.
>
> So the trade off is speed vs potential errors. hmm...

Actually using RtlCreateUnicodeStringFromAsciiz should be faster than
calling MultiByteToWideChar(), HeapAlloc(), MultiByteToWideChar().