Re: RFR: 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files [v4]

2024-01-28 Thread David Holmes
On Sat, 27 Jan 2024 18:24:45 GMT, Coleen Phillimore  wrote:

>> This mechanically replaces NULL with nullptr in hpp/cpp native files in test 
>> native code.  This didn't attempt to change NULL in comments to say null 
>> because nullptr is generally the right thing for the comment to say.  It 
>> does attempt to change NULL to "null" rather than "nullptr" in strings.  Any 
>> changes for "nullptr" to "null" in comments can be changed in a future RFE 
>> in a smaller patch. I didn't see any when it was scrolling by to make my 
>> script more complicated.
>> 
>> Ran tier1-4 testing.
>
> Coleen Phillimore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix one nullptr in comments as found by @kevinw

Looks good. Thanks for doing the text changes as well, they are a necessary 
part of the cleanup.

A number of files are missing copyright updates - the ones I spotted all had a 
single copyright year so maybe your script missed them?

My browser found 21 places where nullptr is cast to something else, which 
should no longer be needed.

Thanks.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17593#pullrequestreview-1847855251


Re: RFR: 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files [v4]

2024-01-28 Thread Kim Barrett
On Sat, 27 Jan 2024 18:24:45 GMT, Coleen Phillimore  wrote:

>> This mechanically replaces NULL with nullptr in hpp/cpp native files in test 
>> native code.  This didn't attempt to change NULL in comments to say null 
>> because nullptr is generally the right thing for the comment to say.  It 
>> does attempt to change NULL to "null" rather than "nullptr" in strings.  Any 
>> changes for "nullptr" to "null" in comments can be changed in a future RFE 
>> in a smaller patch. I didn't see any when it was scrolling by to make my 
>> script more complicated.
>> 
>> Ran tier1-4 testing.
>
> Coleen Phillimore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix one nullptr in comments as found by @kevinw

Looks good.

-

Marked as reviewed by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17593#pullrequestreview-1847625136


Re: RFR: 8324681: Replace NULL with nullptr in HotSpot jtreg test native code files [v4]

2024-01-27 Thread Coleen Phillimore
> This mechanically replaces NULL with nullptr in hpp/cpp native files in test 
> native code.  This didn't attempt to change NULL in comments to say null 
> because nullptr is generally the right thing for the comment to say.  It does 
> attempt to change NULL to "null" rather than "nullptr" in strings.  Any 
> changes for "nullptr" to "null" in comments can be changed in a future RFE in 
> a smaller patch. I didn't see any when it was scrolling by to make my script 
> more complicated.
> 
> Ran tier1-4 testing.

Coleen Phillimore has updated the pull request incrementally with one 
additional commit since the last revision:

  Fix one nullptr in comments as found by @kevinw

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17593/files
  - new: https://git.openjdk.org/jdk/pull/17593/files/33786c7d..6eb051ed

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17593=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=17593=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17593.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17593/head:pull/17593

PR: https://git.openjdk.org/jdk/pull/17593