static declaration inside `window_scroll_pixel_based' in window.c

static declaration inside `window_scroll_pixel_based' in window.c

[Luc Teirlinck]

`(elisp)Writing Emacs Primitives' contains:

     You must not use C initializers for static or global variables unless
  the variables are never written once Emacs is dumped.  These variables
  with initializers are allocated in an area of memory that becomes
  read-only (on certain operating systems) as a result of dumping Emacs.
  *Note Pure Storage::.

     Do not use static variables within functions--place all static
  variables at top level in the file.  This is necessary because Emacs on
  some operating systems defines the keyword `static' as a null macro.
  (This definition is used because those systems put all variables
  declared static in a place that becomes read-only after dumping, whether
  they have initializers or not.)

but preserve_y is declared a static int and initialized to -1 in the
function `window_scroll_pixel_based' on line 4727 of window.c.  I know
there are exceptions to the above quoted rules.  If you are sure that
your code is not going to be used on one of the affected OS's, there
is no problem.  _Maybe_ that applies here, but I am not sure.  Does it?
At first view I would say no, but I have no idea what the problematic
OS's are.

Anyway, if the above quote from `(elisp)Writing Emacs Primitives' does
apply here, the problem would be easy to fix with the following patch,
which I can install if desired.

===File ~/window.c-diff=====================================
*** window.c	28 Feb 2006 10:26:44 -0600	1.537
--- window.c	08 Mar 2006 17:40:13 -0600	
***************
*** 215,220 ****
--- 215,224 ----
  
  int window_deletion_count;
  
+ /* Used by the function window_scroll_pixel_based */
+ 
+ static int preserve_y;
+ 
  #if 0 /* This isn't used anywhere.  */
  /* Nonzero means we can split a frame even if it is "unsplittable".  */
  static int inhibit_frame_unsplittable;
***************
*** 4724,4730 ****
    int this_scroll_margin;
    /* True if we fiddled the window vscroll field without really scrolling.   */
    int vscrolled = 0;
!   static int preserve_y = -1;
  
    SET_TEXT_POS_FROM_MARKER (start, w->start);
  
--- 4728,4735 ----
    int this_scroll_margin;
    /* True if we fiddled the window vscroll field without really scrolling.   */
    int vscrolled = 0;
! 
!   preserve_y = -1;
  
    SET_TEXT_POS_FROM_MARKER (start, w->start);
  
============================================================

Re: static declaration inside `window_scroll_pixel_based' in window.c

[Luc Teirlinck]

>From my earlier message:

   Anyway, if the above quote from `(elisp)Writing Emacs Primitives' does
   apply here, the problem would be easy to fix with the following patch,
   which I can install if desired.

No, I responded too quickly.  The patch is incorrect, since it always
re-initializes preserve_y to -1 at the start of `window_scroll_pixel_based',
defeating its static-ness. 

Sincerely,

Luc.

Re: static declaration inside `window_scroll_pixel_based' in window.c

[Chong Yidong]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

>>From my earlier message:
>
>    Anyway, if the above quote from `(elisp)Writing Emacs Primitives' does
>    apply here, the problem would be easy to fix with the following patch,
>    which I can install if desired.
>
> No, I responded too quickly.  The patch is incorrect, since it always
> re-initializes preserve_y to -1 at the start of `window_scroll_pixel_based',
> defeating its static-ness. 

You can simply initialize it in syms_of_window.

Re: static declaration inside `window_scroll_pixel_based' in window.c

[Luc Teirlinck]

Chong Yidong wrote:

   You can simply initialize it in syms_of_window.

Should I install my patch with that single change then?  I assume that
you received my original message (I myself did not yet receive it back
from Emacs devel) and that the static declaration inside a function
was an oversight, rather than a legitimate exception to the general rule?

Sincerely,

Luc.

Re: static declaration inside `window_scroll_pixel_based' in window.c

[Chong Yidong]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Chong Yidong wrote:
>
>    You can simply initialize it in syms_of_window.
>
> Should I install my patch with that single change then?  I assume that
> you received my original message (I myself did not yet receive it back
> from Emacs devel) and that the static declaration inside a function
> was an oversight, rather than a legitimate exception to the general rule?

I see no reason not to use a global variable, so go ahead and install
it.

Re: static declaration inside `window_scroll_pixel_based' in window.c

[Richard Stallman]

Please install your patch, but also rename the variable to
window_scroll_pixel_based_preserve_y.