Re: [PATCH] Renaming non-X x_* identifiers

["Basil L. Contovounesios" <contovob@tcd.ie>]

Alex Gramiak <agrambot@gmail.com> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>>> diff --git a/src/xdisp.c b/src/xdisp.c
>>> index ae4c405b8d..11667d2735 100644
>>> --- a/src/xdisp.c
>>> +++ b/src/xdisp.c
>>> @@ -11592,9 +11592,8 @@ clear_garbaged_frames (void)
>>>  	      else
>>>  		clear_current_matrices (f);
>>>  
>>> -#if defined (HAVE_WINDOW_SYSTEM) && !defined (HAVE_NS)
>>> -	      x_clear_under_internal_border (f);
>>> -#endif /* HAVE_WINDOW_SYSTEM && !HAVE_NS */
>>> +              if (FRAME_RIF (f)->clear_under_internal_border)
>>> +                FRAME_RIF (f)->clear_under_internal_border (f);
>>>  
>>>  	      fset_redisplay (f);
>>>  	      f->garbaged = false;
>>
>> This causes a segfault in 'emacs -nw' and emacsclient because
>> FRAME_RIF (f) is NULL in tty frames such as the initial daemon frame.
>
> Sorry about the trouble. Somehow I didn't test using -nw with a
> HAVE_WINDOW_SYSTEM build. It should be fixed now.

Thanks for the quick fix!

>> Is x_clear_under_internal_border not supposed to be run in tty frames?
>
> Previously it did, but each implementation of that procedure was a no-op
> for tty frames. The issue is that FRAME_RIF procedures shouldn't be
> called for tty frames; I didn't realize that FRAME_RIF was NULL for tty
> frames instead of a blank struct. I wonder if that should be changed to
> avoid this in the future?

I'm always for consistency, but I'm not familiar enough with this area
to know what drawbacks such a change may entail, or how much of the code
relies on current behaviour.  FWIW, here's the relevant commentary in
termhooks.h:

  /* Window-based redisplay interface for this device (0 for tty
     devices). */
  struct redisplay_interface *rif;

I'm guessing setting it to NULL as opposed to initialising the struct is
an optimisation for tty frames.  As a side note, I chuckled at this
commentary in an old version of dispnew.c:

  /* Current interface for window-based redisplay.  Set from
     update_begin.  A null value means we are not using window-based
     redisplay.  */
  /* XXX this variable causes frequent coredumps */

  struct redisplay_interface *rif;

-- 
Basil