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

Patrick Palka
2015-06-30T14:45:08Z

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
>