On Tue, 6 Jan 2026 19:26:46 GMT, Kevin Walls <[email protected]> wrote:

>> Eunbin Son has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8374341: Update copyright year to 2025 for CoreUtils.java
>
> So that matching has never worked. 8-)
> 
> But if we make this change, and it runs coredumpctl, does this now work as 
> intended?
> I see "No journal files were opened due to insufficient permissions." from 
> coredumpctl (but I didn't test during a test that creates a core).
> 
> Another problem may be  the unpredictability of the timing when files are 
> large:  the 5 second sleep and the 10 iterations.  Is that always enough?  
> Maybe this is fine, but if it has never actually been used, the code should 
> be verified well before it's enabled.
> 
> On my systems when a core dump pattern has the "|", I am familiar with seeing 
> the message "This system uses a crash report tool.." from later in the file.  
> It tells you the system is configured inappropriately, so you fix the pattern 
> manually, and all is good.
> 
> Maybe we should delete lines 168 to 191 and simply have that message at line 
> 192 printed, and a test skipped, if a core file pattern using "|" is in place.

Thank you for the review, @kevinjwalls. You're right that this code has never 
worked, 
and I share your concerns about activating unverified code.

I removed lines 168-191 and keep the skip message at line 192. 
This is safer and simpler. Thank you for the guidance!

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

PR Comment: https://git.openjdk.org/jdk/pull/28984#issuecomment-3718671633

Reply via email to