RFR: 8311542: Consolidate the native stack printing code [v5]

[Julian Waters]

On Wed, 11 Dec 2024 05:47:14 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> src/hotspot/share/runtime/frame.hpp line 443:
>> 
>>> 441:   void print_on_error(outputStream* st, char* buf, int buflen, bool verbose = false) const;
>>> 442:   static void print_C_frame(outputStream* st, char* buf, int buflen, address pc);
>>> 443:   static frame next_frame(frame fr, Thread* t); // For native stack walking
>> 
>> A thought: Does it make more sense to have next_frame implemented in vmError or nativeStackPrinter and made available through their respective headers rather than as a member of the frame class? Just thinking out loud, I'm fine with it as is
>
> I figured the only class that should know that much about how to walk frames should the frame class. In addition it would look strange for either of the other classes to export that kind of API for other classes to use.
> 
> Thanks for re-reviewing. Of course it needs another review now I fixed the indentation issue.

Ah, I was talking about having it as a top level extern method in either nativeStackPrinter or vmError, and declared in their headers for other code to use, not as a member of any of their classes, but I guess it doesn't matter. Thanks for your patience

I don't think my review would've mattered anyway even if you hadn't changed anything since I'm not a Reviewer, but for the sake of it I'll re-approve :)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22472#discussion_r1879387999