<AWT Dev> 8221405: Fix Windows 32bit awt build

[Thomas Stüfe]

Hi Alexey,

On Tue, Mar 26, 2019 at 4:26 PM Alexey Ivanov <alexey.ivanov at oracle.com>
wrote:

> Hi Thomas,
>
> On 26/03/2019 14:41, Thomas Stüfe wrote:
>
> Hi Alexey,
>
>
>
> On Tue, Mar 26, 2019 at 2:36 PM Alexey Ivanov <alexey.ivanov at oracle.com>
> wrote:
>
>> Hi Thomas,
>>
>> Looks good, I'm not a reviewer though.
>> Could I also ask you to push this changeset to jdk/client [1] instead of
>> jdk/jdk?
>>
>> A small question: two prototypes are changed in debug_trace.h but only
>> one is updated in debug_trace.c. Is it because the functions matching
>> the second prototype have already been updated with JNICALL?
>>
>>
> Not really sure I understand the question. There is no real 1:1
> relationship between my change in .h and .c:
>
>
> I mean there are two prototypes in the header:
> 1. typedef void (JNICALL *DTRACE_OUTPUT_CALLBACK)(const char * msg);
> 2. typedef void (JNICALL *DTRACE_PRINT_CALLBACK)(const char * file, int
> line, int argc, const char * fmt, va_list arglist);
>
> 1. In C, only the first one is updated:
> static void JNICALL DTrace_PrintStdErr(const char *msg);
> which corresponds to the first case. This shouldn't have caused build
> failure because the declarations in .h and .c matched.
>
> 2. I believe DTrace_VPrintln and DTrace_VPrint are the callbacks for the
> 2nd prototype of DTRACE_PRINT_CALLBACK. They're received JNICALL under
> JDK-8214120 but the header hasn't been updated. This caused the build
> failure.
>
>
I cannot find any place where function pointers to DTrace_VPrintln or
DTrace_VPrint are used. They only ever are called directly.


> The prevent the build failure, it would be enough to update the 2nd
> declaration in the header. (Or alternatively, drop JNICALL from
> DTrace_VPrintln and DTrace_VPrint in .c file.)
> For consistency, you're updating the 1st declaration as well.
>
>
> DTRACE_PRINTxx macros call _DTrace_Template() which expands to
> DTrace_PrintFunction(). First argument is pointer to DTrace_VPrint(), which
> is a function decorated with JNICALL. Hence the build error - the type of
> the first argument of DTrace_PrintFunction is DTRACE_PRINT_CALLBACK, which
> is a fkt pointer *without* JNICALL.
>
> So one way to fix this was to correct the DTRACE_PRINT_CALLBACK type to be
> a fkt pointer pointing to a JNICALL decorated function.
>
> That means that I also needed to fix all functions whose pointers are
> being passed around as DTRACE_PRINT_CALLBACK. But there only was one I saw,
> DTrace_PrintStdErr().
>
>
> From the above, it looks this isn't necessary as DTrace_PrintStdErr() is
> of type DTRACE_OUTPUT_CALLBACK.
>
> However, I think it's better to have both DTRACE_OUTPUT_CALLBACK and
> DTRACE_PRINT_CALLBACK consistent: either with JNICALL or without.
>
>
> Looks fine to me!
>
>
Great, I pushed. Thanks for reviewing!

--Thomas

> I'll push to jdk/client. Will that be transported to jdk/jdk automatically?
>
>
> Yes, it will.
> Although I'm not sure how often they're synced.
>
> Regards,
> Alexey
>
>
> Cheers, Thomas
>
>
>
>
>
>> Regards,
>> Alexey
>>
>> [1] http://hg.openjdk.java.net/jdk/client/
>>
>> On 25/03/2019 13:09, Thomas Stüfe wrote:
>> > Hi all,
>> >
>> > Issue: https://bugs.openjdk.java.net/browse/JDK-8221405
>> > cr:
>> >
>> http://cr.openjdk.java.net/~stuefe/webrevs/8221405-windows32-awt-buildfixes/webrev.00/webrev/
>> >
>> > On 32bit windows, awt build did bitrot. Dtrace print callbacks need to
>> > be declared with __stdcall. No other platform cares.
>> >
>> > Thanks, Thomas
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/awt-dev/attachments/20190326/f588d8a7/attachment.html>