Re: [PATCH] ucrtbase: Implement t and j printf length modifiers.

[Ambrož Bizjak]

Updated patch:
- Using if (sizeof(type) == 8) instead of #ifdef _WIN64.
- Using the MIN_/MAX_ constants for the exact type being tested to
prevent a warning.
- Added Signed-off-by line.

On Fri, Nov 1, 2019 at 6:02 PM Piotr Caban  wrote:
> Hi,
>
> Can't PTRDIFF_MIN or PTRDIFF_MAX be used to avoid the warning?
>
> Thanks,
> Piotr
>
> On 11/1/19 5:28 PM, Ambrož Bizjak wrote:
> > Hi,
> > The reason for the define is that I was getting warnings about integer
> > overflow from the branch that is not taken (too large value converted to
> > 32 bits).
> >
> > Let me know if that's ok, then I will resend with signed-off.
> >
> > On Friday, November 1, 2019, Piotr Caban <[e-mail]
> > > wrote:
> > Hi Ambrož,
> >
> > The patch looks mostly good to me but it's required that you add
> > signed-off-by line
> > (https://wiki.winehq.org/Submitting_Patches#The_commit_message
> > <https://wiki.winehq.org/Submitting_Patches#The_commit_message>).
> >
> > What's the reason for changing:
> > if (sizeof(void*) == 8)
> > to
> > #ifdef _WIN64
> > ?
> > I think it's better to keep it as is so the code is always compiled.
> >
> > Thanks,
> > Piotr

[PATCH] ucrtbase: Implement t and j printf length modifiers.

[Ambrož Bizjak]

Piotr, the point of the casts was to ensure that the correct argument
type is used. AFAIK it is not guaranteed that the type of PTRDIFF_MIN
is in fact ptrdiff_t.

[PATCH] ucrtbase: Implement t and j printf length modifiers.

[Piotr Caban]

On 11/1/19 8:13 PM, Ambrož Bizjak wrote:
> Piotr, the point of the casts was to ensure that the correct argument
> type is used. AFAIK it is not guaranteed that the type of PTRDIFF_MIN
> is in fact ptrdiff_t.

These casts are not changing anything. And it's guaranteed that 
PTRDIFF_MIN will be passed in the same way as ptrdiff_t as argument.

Thanks,
Piotr

[PATCH] ucrtbase: Implement t and j printf length modifiers.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=58929

Your paranoid android.

=== build (build log) ===

error: patch failed: dlls/msvcrt/printf.h:516
error: patch failed: dlls/ucrtbase/tests/printf.c:24
Task: Patch failed to apply