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

[Kamil Rytarowski <n54@gmx.com>]

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.