[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: comctl32: Avoid magic numbers
From: Nikolay Sivov <bunglehead () gmail ! com>
Date: 2014-10-31 11:41:25
Message-ID: CAG4Z_JJXmTV45Axdx_38qhKWHgGH+5vE84SpFDZ+hC-+rA=E3A () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Fri, Oct 31, 2014 at 2:00 PM, Bruno Jesus <00cpxxx@gmail.com> wrote:
> On Fri, Oct 31, 2014 at 3:11 AM, Nikolay Sivov <nsivov@codeweavers.com>
> wrote:
> >> /* FIXME: This flag is undocumented and unknown by our CompareString.
> >> - * We need a define for it.
> >> */
> >> - dwFlags = 0x10000000;
> >> + dwFlags = LOCALE_RETURN_GENITIVE_NAMES;
> >> if (!bCase) dwFlags |= NORM_IGNORECASE;
> >
> >
> > It makes no sense to use this flag from CompareString(). I think it's
> better
> > to remove it along with this comment.
>
> Well I didn't want to touch that, my logic was that dwFlags was being
> set to LOCALE_USE_CP_ACP and right after that it was set to
> 0x10000000. So I looked at the other flags:
>
> #define LOCALE_NOUSEROVERRIDE 0x80000000
> #define LOCALE_USE_CP_ACP 0x40000000
> #define LOCALE_RETURN_NUMBER 0x20000000
> #define LOCALE_RETURN_GENITIVE_NAMES 0x10000000
>
> And noticed that 0x10000000 is LOCALE_RETURN_GENITIVE_NAMES. This
> patch purpose was solely to remove the magic number. Please feel free
> to remove anything you wish. For the sake of history these commits are
> related to it:
> https://source.winehq.org/git/wine.git/commitdiff/ac323a20
> https://source.winehq.org/git/wine.git/commitdiff/64d68b10
>
>
Sure, I get that. My point was that such knowledge was probably gained by
tracing with native comctl32, and it's not allowed.
Also if Wine doesn't support such flag for CompareString() anyway no need
to bother setting it,
because virtual benefits of running Wine's comctl32 on Windows are
insignificant.
I think I'll send a patch to remove that, yeah. Thanks for bringing this
issue up.
> Best regards,
> Bruno
>
>
>
[Attachment #5 (text/html)]
<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct \
31, 2014 at 2:00 PM, Bruno Jesus <span dir="ltr"><<a \
href="mailto:00cpxxx@gmail.com" target="_blank">00cpxxx@gmail.com</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">On Fri, Oct 31, 2014 at 3:11 AM, Nikolay Sivov <<a \
href="mailto:nsivov@codeweavers.com">nsivov@codeweavers.com</a>> wrote:<br> \
>> /* FIXME: This flag is undocumented and unknown by our \
CompareString.<br> >> - * We need a define for it.<br>
>> */<br>
>> - dwFlags = 0x10000000;<br>
>> + dwFlags = LOCALE_RETURN_GENITIVE_NAMES;<br>
>> if (!bCase) dwFlags |= NORM_IGNORECASE;<br>
><br>
><br>
> It makes no sense to use this flag from CompareString(). I think it's \
better<br> > to remove it along with this comment.<br>
<br>
Well I didn't want to touch that, my logic was that dwFlags was being<br>
set to LOCALE_USE_CP_ACP and right after that it was set to<br>
0x10000000. So I looked at the other flags:<br>
<br>
#define LOCALE_NOUSEROVERRIDE 0x80000000<br>
#define LOCALE_USE_CP_ACP 0x40000000<br>
#define LOCALE_RETURN_NUMBER 0x20000000<br>
#define LOCALE_RETURN_GENITIVE_NAMES 0x10000000<br>
<br>
And noticed that 0x10000000 is LOCALE_RETURN_GENITIVE_NAMES. This<br>
patch purpose was solely to remove the magic number. Please feel free<br>
to remove anything you wish. For the sake of history these commits are<br>
related to it:<br>
<a href="https://source.winehq.org/git/wine.git/commitdiff/ac323a20" \
target="_blank">https://source.winehq.org/git/wine.git/commitdiff/ac323a20</a><br> <a \
href="https://source.winehq.org/git/wine.git/commitdiff/64d68b10" \
target="_blank">https://source.winehq.org/git/wine.git/commitdiff/64d68b10</a><br> \
<br></blockquote><div><br></div><div>Sure, I get that. My point was that such \
knowledge was probably gained by tracing with native comctl32, and it's not \
allowed.</div><div>Also if Wine doesn't support such flag for CompareString() \
anyway no need to bother setting it,</div><div>because virtual benefits of running \
Wine's comctl32 on Windows are insignificant.</div><div><br></div><div>I think \
I'll send a patch to remove that, yeah. Thanks for bringing this issue \
up.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> Best regards,<br>
Bruno<br>
<br>
<br>
</blockquote></div><br></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic