On Fri, 20 Dec 2024 18:58:44 GMT, Phil Race <p...@openjdk.org> wrote:
>> Jan Kratochvil has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains three additional
>> commits since the last revision:
>>
>> - Use only (void) casts
>> - Merge branch 'master' into clangbuild
>> - 8346436: Compilation with clang fails
>
> src/java.desktop/unix/native/libawt_xawt/xawt/XToolkit.c line 696:
>
>> 694: if (!isMainThread() && awt_pipe_inited) {
>> 695: if (write ( AWT_WRITEPIPE, &wakeUp_char, 1 ) != 1) {
>> 696: fprintf(stderr, "Cannot not write to AWT utility control:
>> %s\n", strerror(errno));
>
> what happens to the process if this call fails ?
> Can it continue ?
> You can check by commenting out the call and see what happens when you run
> AWT tests
> And assuming it is a continuable error, can you do the print only in a DEBUG
> build ?
I have verified this `write()` call is exercised by the testsuite
(`test/jdk/java/awt`) but the syscall never fails during my testsuite run. In
the scope of this patch I can also just cast the syscall to `(void)`. I did not
want to do the cast as it will hide an unchecked error path for the future. For
something more I expect one should rather port it to Qt or GTK(GDK) but I do
not think my employer has free capacity for that. I can also change it
according to your directions but (1) I do not have an opinion what it will do
and (2) IMO such engineering is out of scope of this patch. I can also just
cast it to `(void)` and file a JDK issue for that.
> src/java.desktop/unix/native/libsplashscreen/splashscreen_sys.c line 377:
>
>> 375: if (write(splash->controlpipe[1], &code, 1) != 1) {
>> 376: fprintf(stderr, "Cannot not write to splash screen control:
>> %s\n", strerror(errno));
>> 377: }
>
> what happens to the process if this call fails ?
> Can it continue ?
> You can check by commenting out the call and see what happens when you run an
> app on Linux that uses splashScreen ? SwingSet2 would be sufficient.
> And assuming it is a continuable error, can you do the print only in a DEBUG
> build ?
Likewise, the testcase is `test/jdk/java/awt/SplashScreen`.
> src/java.desktop/unix/native/libsplashscreen/splashscreen_sys.c line 716:
>
>> 714: SplashLock(splash);
>> 715: if (pipe(splash->controlpipe)) {
>> 716: fprintf(stderr, "Error creating pipe for splash screen control:
>> %s\n", strerror(errno));
>
> what happens to the process if this call fails ?
> Can it continue ?
> You can check by commenting out the call and see what happens when you run an
> app on Linux that uses splashScreen ? SwingSet2 would be sufficient.
> And assuming it is a continuable error, can you do the print only in a DEBUG
> build ?
Likewise.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22794#discussion_r1901595237
PR Review Comment: https://git.openjdk.org/jdk/pull/22794#discussion_r1901595633
PR Review Comment: https://git.openjdk.org/jdk/pull/22794#discussion_r1901595792
