Re: [PATCH 1/2] Add constructor and destructor to thread_info

[Pedro Alves <palves@redhat.com>]

On 03/28/2017 03:08 PM, Yao Qi wrote:

>>
>> etc.  (and tweak "Non-zero" references to say "True" instead.)
>>
>> (the patch's subject just becomes "C++-fy thread_info a bit" instead.)
>>
> 
> Other thread_info fields can be changed to bool too.  

Yes, hence the "etc.".

> Once I change
> "executing" to bool, the 2nd argument of set_executing should be bool
> too.  I'll change them in separate patches.

Thanks, TBC, I'm fine with incremental progress toward bool
and with doing it opportunistically, rather than requiring targeted
patches.  Otherwise if we require changing everything at once, the
result is that people will avoid doing it, for fear of the growing
cascading work that propagating a type change everywhere tends to require.

> 
> 
>>> @@ -220,7 +205,8 @@ init_thread_list (void)
>>>    for (tp = thread_list; tp; tp = tpnext)
>>>      {
>>>        tpnext = tp->next;
>>> -      free_thread (tp);
>>> +      delete tp;
>>> +
>>
>> Spurious whitespace.
>>
> 
> You mean spurious newline?
> 

Yes, sorry.

>> These could all be in-class initialized like you did to the other
>> fields, instead of using the explicit memset:
>>
>>  - struct btrace_thread_info btrace;
>>  + struct btrace_thread_info btrace {};
>>
>>  - struct target_waitstatus pending_follow;
>>  + struct target_waitstatus pending_follow {TARGET_WAITKIND_SPURIOUS};
>>
> 
> This will only initialize the first member of union
> target_waitstatus.value to zero, so some bits of
> target_waitstatus.value.related_pid is not initialized.

Ah, blah, unions.  Well, the result is that the remaining fields of
the struct end up list->value->zero-initialized.  Zero-initialization
for unions means the first member is zero-initialized, and padding
is initialized to zero.  So I think the end result is the
same anyway.

Thanks,
Pedro Alves