Re: wine/dlls/avifil32: implemented many functions

[Dmitry Timoshkov]

"Michael GЭnnewig" <MichaelGuennewig(a)gmx.de> wrote:
> + LPWSTR szExt = strrchrW(szFile, L'.');
> +#define SLASH(w) ((w) == L'/' || (w) == L'\\')

As were discussed many times using L prefix in order to create unicode
chars/strings is wrong due to difference in size of unicode characters
between windows and unix (2 vs. 4).

We forced to create unicode strings explicitly:
static const WCHAR fooW[] = { 'f','o','o',0 };
static const WCHAR dotW[] = { '.',0 };
static const WCHAR slashW[] = { '/',0 };
static const WCHAR bkslashW[] = { '\\',0 };

That's somewhat ugly and awkward, but it's really needed for
portability.

--
Dmitry.

wine/dlls/avifil32: implemented many functions

"Dmitry Timoshkov" <dmitry(a)baikal.ru> writes:
> "Michael GXnnewig" <MichaelGuennewig(a)gmx.de> wrote:
> > + LPWSTR szExt = strrchrW(szFile, L'.');
> > +#define SLASH(w) ((w) == L'/' || (w) == L'\\')
>
> As were discussed many times using L prefix in order to create unicode
> chars/strings is wrong due to difference in size of unicode characters
> between windows and unix (2 vs. 4).
>
> We forced to create unicode strings explicitly:
> static const WCHAR fooW[] = { 'f','o','o',0 };
> static const WCHAR dotW[] = { '.',0 };
> static const WCHAR slashW[] = { '/',0 };
> static const WCHAR bkslashW[] = { '\\',0 };
>
> That's somewhat ugly and awkward, but it's really needed for
> portability.

But when I change the code to use the "static const WCHAR[]" the
compiler complains:
,-----
| factory.c: In function `AVIFILE_BasenameW':
| factory.c:163: warning: comparison between pointer and integer
| factory.c:163: warning: comparison between pointer and integer
| factory.c:163: warning: comparison between pointer and integer
`-----

So I have adjusted to use this:

static const WCHAR dotW = (WCHAR)'.';

The it compiles and works also for my. Is this okay? When it's I will
change it with my next patch, which still waits to be verified and
be submitted.


  Michael Günnewig

wine/dlls/avifil32: implemented many functions

[Dmitry Timoshkov]

"Michael GЭnnewig" <MichaelGuennewig(a)gmx.de> wrote:
> But when I change the code to use the "static const WCHAR[]" the
> compiler complains:
> ,-----
> | factory.c: In function `AVIFILE_BasenameW':
> | factory.c:163: warning: comparison between pointer and integer
> | factory.c:163: warning: comparison between pointer and integer
> | factory.c:163: warning: comparison between pointer and integer
> `-----
>
> So I have adjusted to use this:
>
> static const WCHAR dotW = (WCHAR)'.';
>
> The it compiles and works also for my. Is this okay?

No.
> When it's I will
> change it with my next patch, which still waits to be verified and
> be submitted.

Look at other places in Wine source how to do it properly.

--
Dmitry.

wine/dlls/avifil32: implemented many functions

"Dmitry Timoshkov" <dmitry(a)baikal.ru> writes:
> "Michael GXnnewig" <MichaelGuennewig(a)gmx.de> wrote:
> > So I have adjusted to use this:
> >
> > static const WCHAR dotW = (WCHAR)'.';
> >
> > The it compiles and works also for my. Is this okay?
>
> No.

Why?
> Look at other places in Wine source how to do it properly.

Okay I have done a

  find . -name '*.c' | xargs fgrep strchrW

and have found these variations:
,-----
| ./controls/menu.c:                      p = strchrW (p + 2, '&');
| ./dlls/comctl32/comctl32undoc.c:  return strchrW(lpStart, wMatch);
| ./dlls/comctl32/comctl32undoc.c:    if( strchrW(lpSet, *(WORD*)lpLoop))
| ./dlls/msvcrt/dir.c:  if ((ptr = strchrW(path, (WCHAR)L':')) != (WCHAR)L'\0')
| ./dlls/shlwapi/url.c:               mp = strchrW(wk1, L'/');
`-----

Variant 1: use simply a char, let compiler auto-convert to WCHAR
Variant 2: use a WORD
Variant 3: use L'.' construct and cast to WCHAR
Variant 4: use L'.' construct

What's the correct way now? Is it documentat anywhere, I haven't found
it in the docu from the CVS.


  Michael Günnewig

wine/dlls/avifil32: implemented many functions

[Dimitrie O. Paun]

On October 13, 2002 08:19 am, Michael Guennewig wrote:
> | ./controls/menu.c:                      p = strchrW (p + 2, '&');

This is OK for strchrW, 'cause it will expand the 
char to a WCHAR automatically.
> | ./dlls/comctl32/comctl32undoc.c:  return strchrW(lpStart, wMatch);
> | ./dlls/comctl32/comctl32undoc.c:    if( strchrW(lpSet, *(WORD*)lpLoop))

WORD is like a WCHAR, it works.
> | ./dlls/msvcrt/dir.c:  if ((ptr = strchrW(path, (WCHAR)L':')) != (WCHAR)L'\0')

Works as well, but I find it unnecessarily complicated.
> | ./dlls/shlwapi/url.c:               mp = strchrW(wk1, L'/');

I say, don't bother with the L, because it means the compiler will have
to do a lossy 32-bit to 16-bit conversion.

I say the first version is just fine (for ASCII chars!).

-- 
Dimi.

wine/dlls/avifil32: implemented many functions

[Alexandre Julliard]

MichaelGuennewig(a)gmx.de (Michael Guennewig) writes:
> Variant 1: use simply a char, let compiler auto-convert to WCHAR
> Variant 2: use a WORD
> Variant 3: use L'.' construct and cast to WCHAR
> Variant 4: use L'.' construct

Variant 1 is best. There is no need to add casts or L prefixes, and it
makes the code harder to read. Of course this only works for 7-bit
ASCII chars, but we shouldn't use anything else in character constants
anyway.

-- 
Alexandre Julliard
julliard(a)winehq.com