Re: [PATCH] Be lazy about refreshing the windows in tui_show_frame_info (PR tui/13378)

[Patrick Palka <patrick@parcs.ath.cx>]

On Tue, Jun 30, 2015 at 10:27 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/30/2015 03:32 AM, Patrick Palka wrote:
>> This revised patch makes sure that tui_set_locator_info returns 1 when the
>> locator is first constructed, just in case none of the later checks trigger
>> for some reason.
>
> I have a couple questions below, but I'm fine with this approach.
>
>> @@ -302,21 +306,36 @@ tui_set_locator_info (struct gdbarch *gdbarch,
>>  {
>>    struct tui_gen_win_info *locator = tui_locator_win_info_ptr ();
>>    struct tui_locator_element *element;
>> +  int locator_changed_p = 0;
>>
>>    /* Allocate the locator content if necessary.  */
>>    if (locator->content_size <= 0)
>>      {
>>        locator->content = tui_alloc_content (1, LOCATOR_WIN);
>>        locator->content_size = 1;
>> +      locator_changed_p = 1;
>>      }
>>
>>    element = &locator->content[0]->which_element.locator;
>> +
>> +  if (procname != NULL)
>> +    locator_changed_p |= strncmp (element->proc_name, procname,
>> +                               MAX_LOCATOR_ELEMENT_LEN) != 0;
>
> Can't element->proc_name be NULL here?

Don't think so, since it is an inline array.  It's defined as:

struct tui_locator_element
{
  ...
  char full_name[MAX_LOCATOR_ELEMENT_LEN];
  char proc_name[MAX_LOCATOR_ELEMENT_LEN];
}

(and tui_alloc_content makes sure to set full_name[0] = proc_name[0] = '\0').

>
> For the string fields, do we also need to compare
> whether we go from NULL <-> non-NULL ?
>
>   locator_changed_p |= ((fullname == NULL) != (element->full_name == NULL));
>
> etc.?

Yeah, that would be more correct I think.  But I think the logic would
have to look like "if (procname == NULL) locator_changed_p |= strlen
(element->proc_name) != 0;" because proc_name cannot be NULL.  When
procname is NULL, proc_name[0] gets set to 0.

>
> Thanks,
> Pedro Alves
>