This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Pedro Alves <palves@redhat.com> writes:

>>    /* Non-zero means the thread is executing.  Note: this is different
>>       from saying that there is an active target and we are stopped at
>>       a breakpoint, for instance.  This is a real indicator whether the
>>       thread is off and running.  */
>> -  int executing;
>> +  int executing = 0;
>
> and do:
>
>   bool executing = false;
>
> 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.  Once I change
"executing" to bool, the 2nd argument of set_executing should be bool
too.  I'll change them in separate patches.


>> @@ -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?

> 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.

> (maybe drop the "struct" too while at it).

-- 
Yao (齐尧)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]