RFR: 8346436: Compilation with clang fails [v2]

[Jan Kratochvil]

On Fri, 20 Dec 2024 18:58:44 GMT, Phil Race <prr at 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