Re: [PATCH v3 2/8] gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap

Thiago Jung Bauermann via Gdb-patches
2023-02-01T21:56:16Z

Simon Marchi <simark@simark.ca> writes:

> On 2/1/23 14:33, Thiago Jung Bauermann via Gdb-patches wrote:
>>
>> Simon Marchi <simon.marchi@efficios.com> writes:
>>
>>>>> @@ -946,17 +946,16 @@ extern int have_ptrace_getregset;
>>>>> *VALP and return 1. If not found or if there is an error, return
>>>>> 0. */
>>>>>
>>>>> -int linux_get_auxv (int wordsize, CORE_ADDR match,
>>>>> - CORE_ADDR *valp);
>>>>> +int linux_get_auxv (int pid, int wordsize, CORE_ADDR match, CORE_ADDR *valp);
>>>>>
>>>>> /* Fetch the AT_HWCAP entry from the auxv vector, where entries are length
>>>>> WORDSIZE. If no entry was found, return zero. */
>>>>>
>>>>> -CORE_ADDR linux_get_hwcap (int wordsize);
>>>>> +CORE_ADDR linux_get_hwcap (int pid, int wordsize);
>>>>>
>>>>> /* Fetch the AT_HWCAP2 entry from the auxv vector, where entries are length
>>>>> WORDSIZE. If no entry was found, return zero. */
>>>>>
>>>>> -CORE_ADDR linux_get_hwcap2 (int wordsize);
>>>>> +CORE_ADDR linux_get_hwcap2 (int pid, int wordsize);
>>>>
>>>> Ideally the comment for these three functions would be updated to
>>>> mention the PID argument.
>>
>> This is how I changed the comments. What do you think?
>>
>> diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
>> index 221de85aa2ee..4f95a27808ba 100644
>> --- a/gdbserver/linux-low.h
>> +++ b/gdbserver/linux-low.h
>> @@ -941,20 +941,19 @@ bool thread_db_thread_handle (ptid_t ptid, gdb_byte **handle, int
>> *handle_len);
>>
>> extern int have_ptrace_getregset;
>>
>> -/* Search for the value with type MATCH in the auxv vector with
>> - entries of length WORDSIZE bytes. If found, store the value in
>> - *VALP and return 1. If not found or if there is an error, return
>> - 0. */
>> +/* Search for the value with type MATCH in the auxv vector, with entries of
>> + length WORDSIZE bytes, of process with PID. If found, store the value
>
> "of process with PID" sounds weird, syntactically. I would say "of process with pid
> PID" (same in the other comments). Otherwise, this is OK.

I adopted this wording and pushed this patch as well as patch 1 in this
series.

Thank you again for your, Andrew's and Luis' reviews on these patches.

--
Thiago