Re: oleaut32: Use BOOL type where appropriate

[Nikolay Sivov]

On 2/22/2014 15:22, Frédéric Delanoy wrote:
> ---
>   dlls/oleaut32/varformat.c | 8 ++++----
>   dlls/oleaut32/vartype.c   | 2 +-
>   2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/dlls/oleaut32/varformat.c b/dlls/oleaut32/varformat.c
> index e72a170..a6a10cf 100644
> --- a/dlls/oleaut32/varformat.c
> +++ b/dlls/oleaut32/varformat.c
> @@ -2303,9 +2303,9 @@ HRESULT WINAPI VarFormatNumber(LPVARIANT pVarIn, INT nDigits, INT nLeading, INT
>       if (nLeading == -2)
>         GETLOCALENUMBER(LOCALE_ILZERO, LeadingZero);
>       else if (nLeading == -1)
> -      numfmt.LeadingZero = 1;
> +      numfmt.LeadingZero = TRUE;
>       else
> -      numfmt.LeadingZero = 0;
> +      numfmt.LeadingZero = FALSE;
>
> if (nGrouping == -2)
>       {
> @@ -2484,9 +2484,9 @@ HRESULT WINAPI VarFormatCurrency(LPVARIANT pVarIn, INT nDigits, INT nLeading,
>       if (nLeading == -2)
>         GETLOCALENUMBER(LOCALE_ILZERO, LeadingZero);
>       else if (nLeading == -1)
> -      numfmt.LeadingZero = 1;
> +      numfmt.LeadingZero = TRUE;
>       else
> -      numfmt.LeadingZero = 0;
> +      numfmt.LeadingZero = FALSE;

That's not a case when it's appropriate - 'LeadingZero' is UINT, not BOOL.
Somewhat improved variant of above conditions could look like that:

---
      else
        numfmt.LeadingZero = nLeading == -1;
---

oleaut32: Use BOOL type where appropriate

[Frédéric Delanoy]

On Sat, Feb 22, 2014 at 12:36 PM, Nikolay Sivov <bunglehead(a)gmail.com> wrote:
> On 2/22/2014 15:22, Frédéric Delanoy wrote:
> > ---
> >   dlls/oleaut32/varformat.c | 8 ++++----
> >   dlls/oleaut32/vartype.c   | 2 +-
> >   2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/dlls/oleaut32/varformat.c b/dlls/oleaut32/varformat.c
> > index e72a170..a6a10cf 100644
> > --- a/dlls/oleaut32/varformat.c
> > +++ b/dlls/oleaut32/varformat.c
> > @@ -2303,9 +2303,9 @@ HRESULT WINAPI VarFormatNumber(LPVARIANT pVarIn, INT
> > nDigits, INT nLeading, INT
> >       if (nLeading == -2)
> >         GETLOCALENUMBER(LOCALE_ILZERO, LeadingZero);
> >       else if (nLeading == -1)
> > -      numfmt.LeadingZero = 1;
> > +      numfmt.LeadingZero = TRUE;
> >       else
> > -      numfmt.LeadingZero = 0;
> > +      numfmt.LeadingZero = FALSE;
> >         if (nGrouping == -2)
> >       {
> > @@ -2484,9 +2484,9 @@ HRESULT WINAPI VarFormatCurrency(LPVARIANT pVarIn,
> > INT nDigits, INT nLeading,
> >       if (nLeading == -2)
> >         GETLOCALENUMBER(LOCALE_ILZERO, LeadingZero);
> >       else if (nLeading == -1)
> > -      numfmt.LeadingZero = 1;
> > +      numfmt.LeadingZero = TRUE;
> >       else
> > -      numfmt.LeadingZero = 0;
> > +      numfmt.LeadingZero = FALSE;
>
> That's not a case when it's appropriate - 'LeadingZero' is UINT, not BOOL.
> Somewhat improved variant of above conditions could look like that:
>
> ---
>      else
>        numfmt.LeadingZero = nLeading == -1;

According to MSDN, it's either 0 for non-leading zero, or 1 for
leading.zero (hence TRUE/FALSE def'd to 1/0 in windef)
Now obviously the field type can't be modified, but in other places in
the code, booleans are stored e.g. in unsigned foo:1, or other
unsigned type.

cf.
http://msdn.microsoft.com/en-us/library/windows/desktop/dd319095%28v=vs.85%2...
http://msdn.microsoft.com/en-us/library/windows/desktop/dd373784%28v=vs.85%2...

Frédéric

oleaut32: Use BOOL type where appropriate

[Ken Thomases]

On Feb 22, 2014, at 11:14 AM, Frédéric Delanoy wrote:
> According to MSDN, it's either 0 for non-leading zero, or 1 for
> leading.zero

If the documented values are 0 or 1 (rather than, say, FALSE or TRUE), then why are you changing 0s and 1s to something else?
> (hence TRUE/FALSE def'd to 1/0 in windef)

"Hence" implies a causal relationship.  The fact that TRUE and FALSE happen to be defined to 1 and 0 is unrelated to the fact that LeadingZero should be either 1 or 0.

-Ken

oleaut32: Use BOOL type where appropriate

[Frédéric Delanoy]

On Sat, Feb 22, 2014 at 6:39 PM, Ken Thomases <ken(a)codeweavers.com> wrote:
> On Feb 22, 2014, at 11:14 AM, Frédéric Delanoy wrote:
> > According to MSDN, it's either 0 for non-leading zero, or 1 for
> > leading.zero
>
> If the documented values are 0 or 1 (rather than, say, FALSE or TRUE), then why are you changing 0s and 1s to something else?
> > (hence TRUE/FALSE def'd to 1/0 in windef)
>
> "Hence" implies a causal relationship.  The fact that TRUE and FALSE happen to be defined to 1 and 0 is unrelated to the fact that LeadingZero should be either 1 or 0.

yeah, my sentence was a bit ambiguous/unclear, I meant "hence using
T/F, which are defined to 1/0 in windef)." Sorry about that.
> -Ken

The LeadingZero conveys itself a boolean/logical semantics (i.e. has
leading zeros or not), and it would be clearer to assign truths values
to it IMHO (and it's not like T/F definitions are ever about to
change).
But of course I can understand your line of reasoning as well.

Frédéric Delanoy