On Tue, 22 Apr 2025 09:28:52 GMT, Kevin Walls <[email protected]> wrote:
>> Daniel Jeliński has updated the pull request incrementally with eight
>> additional commits since the last revision:
>>
>> - Update copyright
>> - Add more error messages
>> - Add more error messages
>> - Add more error messages
>> - Add more error messages
>> - Add more error messages
>> - Add more error messages
>> - Add more error messages
>
> src/jdk.hotspot.agent/linux/native/libsaproc/ps_proc.c line 450:
>
>> 448: if ( (ph = (struct ps_prochandle*) calloc(1, sizeof(struct
>> ps_prochandle))) == NULL) {
>> 449: snprintf(err_buf, err_buf_len, "can't allocate memory for
>> ps_prochandle");
>> 450: print_error("%s\n", err_buf);
>
> This might be one that would print it twice - the caller in
> LinuxDebuggerLocal.cpp will print what's in err_buf.
Looks like here in Pgrab the intent is to snprintf an error message in err_buf,
and the caller prints it. So Pgrab should not need the print_error calls.
Some functions which are called, like read_lib_info, may print their own
specific error message, then Pgrab stores an error that "failed to read lib
info".. These are reporting the same thing but I don't see that as a problem or
duplication, it could be really useful to track where things fail.
Yes it's a little annoying that Pgrab_core is different, it doesn't take an
error_buf so needs to print_error about any problems.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24722#discussion_r2053763445
