Re: [PATCH] Unicode Lisp reader escapes

[Aidan Kehoe <kehoea@parhasard.net>]

 Ar an t-ochtú lá de mí Bealtaine, scríobh Kenichi Handa: 

 > >> > readcharfun is declared as a Lisp_Object in read1, so it should be
 > >> > possible to check it's type and only GCPRO when necessary.
 > >> 
 > >> I don't see any need to GCPRO readcharfun. When called from Lisp the
 > >> arguments are already protected by being part of the call frame, and
 > >> all uses from C protect the object by other means (eg, by being put on
 > >> eval-buffer-list).
 > 
 > > That was my understanding of the code too. 
 > 
 > For instance, Fread is called from Fcall_interactively as
 > below:
 > 
 > 		Lisp_Object tem;
 > [...]
 > 		tem = Fread_from_minibuffer (build_string (callint_message),
 > 					     Qnil, Qnil, Qnil, Qnil, Qnil,
 > 					     Qnil, Qnil);
 > 		if (! STRINGP (tem) || SCHARS (tem) == 0)
 > 		  args[i] = Qnil;
 > 		else
 > 		  args[i] = Fread (tem);
 > 
 > In the calling sequence of Fread->read_internal_start->read0->read1, I
 > see no place where the original `tem' is GCPROed. Do I overlook
 > something?

I believe not, it does need to be protected. 

Also, my understanding of the above code is that build_string allocates
memory for a Lisp string, that is not visible from Lisp, and that will not
be GCPROed. So if garbage collection happens during Fread_from_minibuffer,
it may disappear. Ben Wing, in the XEmacs internals manual, says this: 

12. Be careful of traps, like calling `Fcons()' in the argument to another
function.  By the "caller protects" law, you should be `GCPRO'ing the
newly-created cons, but you aren't.  A certain number of functions that are
commonly called on freshly created stuff (e.g. `nconc2()', `Fsignal()')
break the "caller protects" law and go ahead and `GCPRO' their arguments so
as to simplify thngs, but make sure and check if it's OK whenever doing
something like this. 

This seems to me equivalent to calling Fcons in the argument to another
function. Is GNU Emacs different in this?

-- 
Aidan Kehoe, http://www.parhasard.net/