make-indirect-buffer

make-indirect-buffer

[Luc Teirlinck]

I noticed a second (less serious, but still confusing) bug in
`make-indirect-buffer'.

After emacs -q, we run ielm:

===File ~/namebug===========================================
*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (make-indirect-buffer "nosuchbuffer" "indi")
*** Eval error ***  No such buffer: `indi'
ELISP> ============================================================

There is not supposed to be an `indi' buffer, we are trying to
construct one.  The problem is that "nosuchbuffer" does not exist.

After the following patch:

===File ~/buffer.c-newdiff==================================
*** buffer.c	11 Apr 2004 10:08:04 -0500	1.448
--- buffer.c	11 Apr 2004 23:41:23 -0500	
***************
*** 536,544 ****
    if (!NILP (buf))
      error ("Buffer name `%s' is in use", SDATA (name));
  
    base_buffer = Fget_buffer (base_buffer);
!   if (NILP (base_buffer))
!     error ("No such buffer: `%s'", SDATA (name));
  
    if (SCHARS (name) == 0)
      error ("Empty string for buffer name is not allowed");
--- 536,547 ----
    if (!NILP (buf))
      error ("Buffer name `%s' is in use", SDATA (name));
  
+   if (NILP (Fget_buffer (base_buffer)))
+     error ("No such buffer: `%s'", SDATA (base_buffer));
+ 
    base_buffer = Fget_buffer (base_buffer);
!   if (NILP (XBUFFER (base_buffer)->name))
!     error ("Base buffer has been killed");
  
    if (SCHARS (name) == 0)
      error ("Empty string for buffer name is not allowed");
============================================================

We get:

===File ~/namebug-after-patch===============================
*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (make-indirect-buffer "nosuchbuffer" "indi")
*** Eval error ***  No such buffer: `nosuchbuffer'
ELISP> 
============================================================

Note that base_buffer could be either an existing buffer or a string,
but if `Fget_buffer (base_buffer)' returns nil, it has to be a string.
I could install if desired.

Sincerely,

Luc.

Re: make-indirect-buffer

[Stefan Monnier]

> ***************
> *** 536,544 ****
>     if (!NILP (buf))
>       error ("Buffer name `%s' is in use", SDATA (name));
  
>     base_buffer = Fget_buffer (base_buffer);
> !   if (NILP (base_buffer))
> !     error ("No such buffer: `%s'", SDATA (name));
  
>     if (SCHARS (name) == 0)
>       error ("Empty string for buffer name is not allowed");
> --- 536,547 ----
>     if (!NILP (buf))
>       error ("Buffer name `%s' is in use", SDATA (name));
  
> +   if (NILP (Fget_buffer (base_buffer)))
> +     error ("No such buffer: `%s'", SDATA (base_buffer));
> + 
>     base_buffer = Fget_buffer (base_buffer);
> !   if (NILP (XBUFFER (base_buffer)->name))
> !     error ("Base buffer has been killed");
  
>     if (SCHARS (name) == 0)
>       error ("Empty string for buffer name is not allowed");

The basic idea looks fine, but the above code calls Fget_buffer twice for
no reason.  It seems the following code would work as well:

   base_buffer = Fget_buffer (base_buffer);
   if (NILP (base_buffer))
     /* We assume that (get-buffer foo)==nil implies STRINGP (foo).
        Is that true?  --Stef  */
     error ("No such buffer: `%s'", SDATA (base_buffer));
   if (NILP (XBUFFER (base_buffer)->name))
     error ("Base buffer has been killed");


-- Stefan

Re: make-indirect-buffer

[Miles Bader]

On Mon, Apr 12, 2004 at 05:01:09PM -0400, Stefan Monnier wrote:
> The basic idea looks fine, but the above code calls Fget_buffer twice for
> no reason.  It seems the following code would work as well:
> 
>    base_buffer = Fget_buffer (base_buffer);
>    if (NILP (base_buffer))
>      /* We assume that (get-buffer foo)==nil implies STRINGP (foo).
>         Is that true?  --Stef  */
>      error ("No such buffer: `%s'", SDATA (base_buffer));

The error message is different -- you're treating NIL as a string...

It seems to avoid calling Fget_buffer twice, you'd have to save the previous
value of base_buffer and use it in the error message.

-Miles
-- 
Occam's razor split hairs so well, I bought the whole argument!

Re: make-indirect-buffer

[Luc Teirlinck]

I erased the CC'c in my previous message:

Miles Bader wrote:

   The error message is different -- you're treating NIL as a
   string...

   It seems to avoid calling Fget_buffer twice, you'd have to save the
   previous
   value of base_buffer and use it in the error message.

Yes.  The question is whether calling Fget_buffer is expensive enough
to justify another variable.

Sincerely,

Luc.

Re: make-indirect-buffer

[Luc Teirlinck]

Basically one could do this (I do not know whether it is worth the
extra variable):

{
  Lisp_Object buf;
  Lisp_Object tem;
  struct buffer *b;

  CHECK_STRING (name);
  buf = Fget_buffer (name);
  if (!NILP (buf))
    error ("Buffer name `%s' is in use", SDATA (name));

  tem = base_buffer;
  base_buffer = Fget_buffer (base_buffer);
  if (NILP (Fget_buffer (base_buffer)))
    error ("No such buffer: `%s'", SDATA (tem));
  if (NILP (XBUFFER (base_buffer)->name))
    error ("Base buffer has been killed");

Re: make-indirect-buffer

[Luc Teirlinck]

>From my previous message:

     tem = base_buffer;
     base_buffer = Fget_buffer (base_buffer);
     if (NILP (Fget_buffer (base_buffer)))
       error ("No such buffer: `%s'", SDATA (tem));

Obviously, I meant:

     tem = base_buffer;
     base_buffer = Fget_buffer (base_buffer);
     if (NILP base_buffer)
       error ("No such buffer: `%s'", SDATA (tem));


Sincerely,

Luc.

Re: make-indirect-buffer

[Luc Teirlinck]

What I really obviously meant was:

     tem = base_buffer;
     base_buffer = Fget_buffer (base_buffer);
     if (NILP (base_buffer))
       error ("No such buffer: `%s'", SDATA (tem));

Re: make-indirect-buffer

[Stefan Monnier]

> Basically one could do this (I do not know whether it is worth the
> extra variable):

AFAIK, variables have no cost.


        Stefan

Re: make-indirect-buffer

[Luc Teirlinck]

Stefan Monnier wrote:

   > Basically one could do this (I do not know whether it is worth the
   > extra variable):

   AFAIK, variables have no cost.

So what if I install the following:

===File ~/buffer.c-diff=====================================
diff -c /home/teirllm/emacscvsdir/emacs/src/buffer.c /home/teirllm/buffer.new.c
*** /home/teirllm/emacscvsdir/emacs/src/buffer.c	Mon Apr 12 18:26:33 2004
--- /home/teirllm/buffer.new.c	Mon Apr 12 18:01:22 2004
***************
*** 521,527 ****
         2, 3,
         "bMake indirect buffer (to buffer): \nBName of indirect buffer: ",
         doc: /* Create and return an indirect buffer for buffer BASE-BUFFER, named NAME.
! BASE-BUFFER should be an existing buffer (or buffer name).
  NAME should be a string which is not the name of an existing buffer.
  Optional argument CLONE non-nil means preserve BASE-BUFFER's state,
  such as major and minor modes, in the indirect buffer.
--- 521,527 ----
         2, 3,
         "bMake indirect buffer (to buffer): \nBName of indirect buffer: ",
         doc: /* Create and return an indirect buffer for buffer BASE-BUFFER, named NAME.
! BASE-BUFFER should be a live buffer, or the name of an existing buffer.
  NAME should be a string which is not the name of an existing buffer.
  Optional argument CLONE non-nil means preserve BASE-BUFFER's state,
  such as major and minor modes, in the indirect buffer.
***************
*** 529,535 ****
       (base_buffer, name, clone)
       Lisp_Object base_buffer, name, clone;
  {
!   Lisp_Object buf;
    struct buffer *b;
  
    CHECK_STRING (name);
--- 529,535 ----
       (base_buffer, name, clone)
       Lisp_Object base_buffer, name, clone;
  {
!   Lisp_Object buf, tem;
    struct buffer *b;
  
    CHECK_STRING (name);
***************
*** 537,545 ****
    if (!NILP (buf))
      error ("Buffer name `%s' is in use", SDATA (name));
  
    base_buffer = Fget_buffer (base_buffer);
    if (NILP (base_buffer))
!     error ("No such buffer: `%s'", SDATA (name));
  
    if (SCHARS (name) == 0)
      error ("Empty string for buffer name is not allowed");
--- 537,548 ----
    if (!NILP (buf))
      error ("Buffer name `%s' is in use", SDATA (name));
  
+   tem = base_buffer;
    base_buffer = Fget_buffer (base_buffer);
    if (NILP (base_buffer))
!     error ("No such buffer: `%s'", SDATA (tem));
!   if (NILP (XBUFFER (base_buffer)->name))
!     error ("Base buffer has been killed");
  
    if (SCHARS (name) == 0)
      error ("Empty string for buffer name is not allowed");

Diff finished.  Mon Apr 12 18:27:39 2004
============================================================

Re: make-indirect-buffer

[Luc Teirlinck]

Stefan Monnier wrote:

	/* We assume that (get-buffer foo)==nil implies STRINGP (foo).
	   Is that true?  --Stef  */

Yes:

DEFUN ("get-buffer", Fget_buffer, Sget_buffer, 1, 1, 0,
       doc: /* Return the buffer named NAME (a string).
If there is no live buffer named NAME, return nil.
NAME may also be a buffer; if so, the value is that buffer.  */)
     (name)
     register Lisp_Object name;
{
  if (BUFFERP (name))
    return name;
  CHECK_STRING (name);

  return Fcdr (assoc_ignore_text_properties (name, Vbuffer_alist));
}

nil is not a buffer.  So the first "return" can not yield nil.

Then comes: CHECK_STRING (name);

Sincerely,

Luc.

Re: make-indirect-buffer

[Kim F. Storm]

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

> I could install if desired.

I think your patches make a lot of sense.  I'd say go for it.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: make-indirect-buffer

[Richard Stallman]

    There is not supposed to be an `indi' buffer, we are trying to
    construct one.  The problem is that "nosuchbuffer" does not exist.

    After the following patch:

Please install it.

make-indirect-buffer

[Luc Teirlinck]

Executing the following ielm run after `emacs -q' makes Emacs crash:

===File ~/indi-after-crash==================================
*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (setq apr (get-buffer-create "april"))
#<buffer april>
ELISP> (with-current-buffer apr (insert "123"))
nil
ELISP> (kill-buffer apr)
t
ELISP> (make-indirect-buffer apr "indi")
#<buffer indi>
ELISP> (buffer-base-buffer (get-buffer "indi"))
#<killed buffer>
ELISP> (buffer-size (get-buffer "indi"))
3
ELISP> (switch-to-buffer "indi")
#<buffer indi>
ELISP> 
============================================================

(The last two lines are from the auto-save file.)

I do not understand how it could possibly be useful to make an
indirect buffer with a killed base buffer, especially since killing a
base buffer automatically kills all indirect buffers.  So my solution
is to throw an error if one tries to make an indirect buffer with
killed base buffer.  The following trivial patch would do that.  I
could install if desired.

===File ~/buffer.c-diff=====================================
*** buffer.c	11 Apr 2004 10:08:04 -0500	1.448
--- buffer.c	11 Apr 2004 23:08:34 -0500	
***************
*** 539,544 ****
--- 539,546 ----
    base_buffer = Fget_buffer (base_buffer);
    if (NILP (base_buffer))
      error ("No such buffer: `%s'", SDATA (name));
+   if (NILP (XBUFFER (base_buffer)->name))
+     error ("Base buffer has been killed");
  
    if (SCHARS (name) == 0)
      error ("Empty string for buffer name is not allowed");
============================================================

After the patch we get:

===File ~/indi-after-patch==================================
*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (setq apr (get-buffer-create "april"))
#<buffer april>
ELISP> (with-current-buffer apr (insert "123"))
nil
ELISP> (kill-buffer apr)
t
ELISP> (make-indirect-buffer apr "indi")
*** Eval error ***  Base buffer has been killed
ELISP> (make-indirect-buffer apr "indi" t)
*** Eval error ***  Base buffer has been killed
ELISP> 
============================================================

Re: make-indirect-buffer

[Richard Stallman]

      So my solution
    is to throw an error if one tries to make an indirect buffer with
    killed base buffer.  The following trivial patch would do that.

I agree with you; please install it.

make-indirect-buffer

[Luc Teirlinck]

I am currently checking buffers.texi.  The CLONE argument to
`make-indirect-buffer' is currently not documented in the Elisp
manual.  No problem there, I will document it.  However, I do not want
to document bugs and I believe the behavior illustrated in the ielm
run below is a bug.  I believe that the intent of the CLONE argument
to `make-indirect-buffer' is to _initialize_ the "state" of the
indirect buffer from the base buffer, but nevertheless return a "true"
indirect buffer, with its own buffer-local-variables.  That would be a
useful thing to do.  What happens instead is that we wind up with shared
local variables, whose value when set in either buffer also affects
the value in the other.  (On the other hand, newly created local
variables are _not_ shared in this way.)  I believe this is a bug and
that no local variables should be "shared" in that fashion.
Ielm run illustrating this:

===File ~/indi-ielm=========================================
*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (set-buffer "*scratch*")
#<buffer *scratch*>
ELISP> (make-local-variable 'aa)
aa
ELISP> (setq aa 1)
1
ELISP> (make-indirect-buffer "*scratch*" "clone" t)
#<buffer clone>
ELISP> (set-buffer "clone")
#<buffer clone>
ELISP> (make-local-variable 'bb)
bb
ELISP> (setq aa 2)
2
ELISP> (setq bb 2)
2
ELISP> (set-buffer "*scratch*")
#<buffer *scratch*>
ELISP> aa
2  ;;  This is what I believe is a bug.  I expected `1'.
ELISP> bb
*** Eval error ***  Symbol's value as variable is void: bb
ELISP> (default-value 'aa)
*** Eval error ***  Symbol's value as variable is void: aa
ELISP> ============================================================

Doc string of `make-indirect-buffer':

make-indirect-buffer is an interactive built-in function.
(make-indirect-buffer BASE-BUFFER NAME &optional CLONE)

Create and return an indirect buffer for buffer BASE-BUFFER, named NAME.
BASE-BUFFER should be an existing buffer (or buffer name).
NAME should be a string which is not the name of an existing buffer.
Optional argument CLONE non-nil means preserve BASE-BUFFER's state,
such as major and minor modes, in the indirect buffer.
CLONE nil means the indirect buffer's state is reset to default values.

Re: make-indirect-buffer

[Richard Stallman]

      What happens instead is that we wind up with shared
    local variables, whose value when set in either buffer also affects
    the value in the other.

That is definitely a bug.  Does this fix it correctly?


*** buffer.c.~1.445.~	Mon Feb 16 16:22:55 2004
--- buffer.c	Sun Mar 14 23:01:12 2004
***************
*** 487,493 ****
  clone_per_buffer_values (from, to)
       struct buffer *from, *to;
  {
!   Lisp_Object to_buffer;
    int offset;
  
    XSETBUFFER (to_buffer, to);
--- 487,493 ----
  clone_per_buffer_values (from, to)
       struct buffer *from, *to;
  {
!   Lisp_Object to_buffer, tem;
    int offset;
  
    XSETBUFFER (to_buffer, to);
***************
*** 514,519 ****
--- 514,527 ----
  
    to->overlays_before = copy_overlays (to, from->overlays_before);
    to->overlays_after = copy_overlays (to, from->overlays_after);
+ 
+   /* Copy the alist of local variables,
+      and all the alist elements too.  */
+   to->local_var_alist
+     = Fcopy_sequence (from->local_var_alist);
+   for (tem = to->local_var_alist; CONSP (tem);
+        tem = XCDR (tem))
+     XSETCAR (tem, Fcons (XCAR (XCAR (tem)), XCDR (XCAR (tem))));
  }

Re: make-indirect-buffer

[Luc Teirlinck]

Richard Stallman wrote:

	 What happens instead is that we wind up with shared
       local variables, whose value when set in either buffer also affects
       the value in the other.

   That is definitely a bug.  Does this fix it correctly?

Yes it does,

Sincerely,

Luc.