Re: [PATCH v4] Implement basic threading support in the NetBSD target

Kamil Rytarowski
2020-04-02T19:24:10Z

On 02.04.2020 21:05, Pedro Alves wrote:
> Hi,
>
> I skimmed the patch and noticed a couple nits.
>
> On 4/1/20 10:33 PM, Kamil Rytarowski wrote:
>
>> +static bool
>> +nbsd_thread_lister (const pid_t pid,
>> + gdb::function_view<bool (const struct kinfo_lwp *)>
>> + callback)
>> +{
>> + int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
>> + size_t size;
>> +
>> + if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
>> + perror_with_name (("sysctl"));
>> +
>> + mib[4] = size / sizeof (size_t);
>> +
>> + gdb::unique_xmalloc_ptr<struct kinfo_lwp> kl
>
> You can use the array variant:
>
> gdb::unique_xmalloc_ptr<struct kinfo_lwp[]>
>

Done.

>> + ((struct kinfo_lwp *) xcalloc (size, 1));
>> + if (kl == NULL)
>> + perror_with_name (("calloc"));
>
> xcalloc doesn't ever return NULL.
>

Done.

>> +
>> + if (sysctl (mib, ARRAY_SIZE (mib), kl.get (), &size, NULL, 0) == -1
>> + || size == 0)
>> + perror_with_name (("sysctl"));
>> +
>> + for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
>> + {
>> + struct kinfo_lwp *l = &kl.get ()[i];
>
> If you use the array variant, then here you don't need the .get(), like:
>
> struct kinfo_lwp *l = &kl[i];
>

Done.

> Thanks,
> Pedro Alves
>

Please see v5.