Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
On Tue, 24 Nov 2020 15:38:17 GMT, Maurizio Cimadamore wrote: >> Marked as reviewed by chegar (Reviewer). > > Following @dholmes-ora suggestion and also @jddarcy (in CSR review), I've > uploaded a new iteration which instead of adding a `@throws` tag at every > method, adds a general statement on each class which gets the treatment. I > think specifying at package-level seems a bit too loose, so I went for > per-class basis. specdiff: http://cr.openjdk.java.net/~mcimadamore/8256865_v2/specdiff_out/overview-summary.html javadoc: http://cr.openjdk.java.net/~mcimadamore/8256865_v2/javadoc - PR: https://git.openjdk.java.net/jdk/pull/1388
Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
On Tue, 24 Nov 2020 09:34:09 GMT, Chris Hegarty wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix cut/paste error in FunctionDescriptor > > Marked as reviewed by chegar (Reviewer). Following @dholmes-ora suggestion and also @jddarcy (in CSR review), I've uploaded a new iteration which instead of adding a `@throws` tag at every method, adds a general statement on each class which gets the treatment. I think specifying at package-level seems a bit too loose, so I went for per-class basis. - PR: https://git.openjdk.java.net/jdk/pull/1388
Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
On Mon, 23 Nov 2020 18:22:14 GMT, Maurizio Cimadamore wrote: >> Both the Foreign Memory Access and the Foreign Linker APIs leave something >> to be desired when it comes to handling NPEs - first, most of the API >> javadoc is oblivious to NPEs being thrown. Secondly, not all API method >> implementations add expicit NPE checks - with the result of NPE often being >> thrown very deep in the call chain - if at all. Third, test for API coverage >> of nulls is ad-hoc. >> >> This patch rectifies all these issues. To increase coverage for null >> injected into APIs, this patch introduces a new framework for testing an API >> in bulk, so that all methods are reflectively called with some values >> replaced with nulls, so that all combinations are tried. >> >> I've also added, as part of this patch, a test to cover the statics in >> MemoryAccess which were not covered throughly. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix cut/paste error in FunctionDescriptor Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1388
Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
On Mon, 23 Nov 2020 18:22:14 GMT, Maurizio Cimadamore wrote: >> Both the Foreign Memory Access and the Foreign Linker APIs leave something >> to be desired when it comes to handling NPEs - first, most of the API >> javadoc is oblivious to NPEs being thrown. Secondly, not all API method >> implementations add expicit NPE checks - with the result of NPE often being >> thrown very deep in the call chain - if at all. Third, test for API coverage >> of nulls is ad-hoc. >> >> This patch rectifies all these issues. To increase coverage for null >> injected into APIs, this patch introduces a new framework for testing an API >> in bulk, so that all methods are reflectively called with some values >> replaced with nulls, so that all combinations are tried. >> >> I've also added, as part of this patch, a test to cover the statics in >> MemoryAccess which were not covered throughly. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix cut/paste error in FunctionDescriptor Unused imports in TestLayouts.java? - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1388
Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
On Mon, 23 Nov 2020 18:10:47 GMT, Jorn Vernee wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix cut/paste error in FunctionDescriptor > > Already looked at this in panama-foreign, but found one minor issue. Just an observation but there are precedents for just placing a generic statement in the package and/or class javadoc: "Unless specified otherwise any method that is passed a null reference will throw NullPointerException" rather than adding @throws NPE all over the place. - PR: https://git.openjdk.java.net/jdk/pull/1388
Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
> Both the Foreign Memory Access and the Foreign Linker APIs leave something to > be desired when it comes to handling NPEs - first, most of the API javadoc is > oblivious to NPEs being thrown. Secondly, not all API method implementations > add expicit NPE checks - with the result of NPE often being thrown very deep > in the call chain - if at all. Third, test for API coverage of nulls is > ad-hoc. > > This patch rectifies all these issues. To increase coverage for null injected > into APIs, this patch introduces a new framework for testing an API in bulk, > so that all methods are reflectively called with some values replaced with > nulls, so that all combinations are tried. > > I've also added, as part of this patch, a test to cover the statics in > MemoryAccess which were not covered throughly. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix cut/paste error in FunctionDescriptor - Changes: - all: https://git.openjdk.java.net/jdk/pull/1388/files - new: https://git.openjdk.java.net/jdk/pull/1388/files/80ffde44..4565f777 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1388=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1388=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1388.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1388/head:pull/1388 PR: https://git.openjdk.java.net/jdk/pull/1388
Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
On Mon, 23 Nov 2020 17:58:22 GMT, Jorn Vernee wrote:
>> Maurizio Cimadamore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix cut/paste error in FunctionDescriptor
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/FunctionDescriptor.java
> line 152:
>
>> 150: * @return the new function descriptor.
>> 151: * @throws NullPointerException if either {@code addedLayouts ==
>> null}, or any of the
>> 152: * layouts in {@code addedLayouts} is null.@throws
>> NullPointerException if any of the new argument layouts is null.
>
> Spurious `@throws` tag here
Good catch
-
PR: https://git.openjdk.java.net/jdk/pull/1388
