RFR: 8343802: Prevent NULL usage backsliding [v5]

Andrey Turbanov aturbanov at openjdk.org
Sun Feb 9 15:13:16 UTC 2025


On Fri, 7 Feb 2025 17:10:59 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:

>> Please review this patch to add a test that checks the hotspot sources and test files for usages of NULL.
>> It scans files in those directories, filtering out certain files as well as all `.c`, `.java`, `.class`, `.jar` and `.zip` files in test sources. 
>> 
>> Before adding line 86 and excluding `os_windows.cpp`, the test failed with:
>> 
>> 
>> Error: 'NULL' found in /w/jdk/src/hotspot/os/windows/os_windows.cpp at line 4436:
>>     HMODULE hModule = NULL;
>> Error: 'NULL' found in /w/jdk/src/hotspot/os/windows/os_windows.cpp at line 4437:
>>     GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, NULL, &hModule);
>> java.lang.RuntimeException: Found usage of 'NULL' in source files. See errors above.
>> 	at TestNoNULL.main(TestNoNULL.java:73)
>> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>> 	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
>> 	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
>> 	at java.base/java.lang.Thread.run(Thread.java:1447)
>
> Nizar Benalla 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 six additional commits since the last revision:
> 
>  - filter out `.zip` files
>  - Merge remote-tracking branch 'upstream/master' into NULL-Checking-in-hotspot
>  - trivial change, if .java files are filtered out then so should .class files
>  - revert to the original regex and remove the exclusion of os_windows.cpp
>  - update based on feedback
>  - Add a test to prevent NULL backsliding

test/hotspot/jtreg/sources/TestNoNULL.java line 67:

> 65:             processFiles(srcPath, excludedSourceFiles, false);
> 66:         }
> 67:         processFiles(testPath, excludedTestFiles, true);

I think it would clearer to pass not the boolean flag, but Set `excludedTestExtensions` itself. And pass empty set for `srcPath`.

        if (Files.exists(srcPath)) {
            processFiles(srcPath, excludedSourceFiles, Set.of());
        }
        processFiles(testPath, excludedTestFiles, excludedTestExtensions);

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23466#discussion_r1948129178


More information about the hotspot-dev mailing list