[bug] read-passwd of CVS Emacs

[bug] read-passwd of CVS Emacs

[Kazu Yamamoto]

Hello,

If the CONFIRM argument is specified to read-passwd of CVS Emacs, 
it causes an error after a user type a first password.

	(read-passwd "password: " t)

--Kazu

Re: [bug] read-passwd of CVS Emacs

[Kevin Rodgers]

Kazu Yamamoto (山本和彦) wrote:
> If the CONFIRM argument is specified to read-passwd of CVS Emacs, 
> it causes an error after a user type a first password.
> 
> 	(read-passwd "password: " t)

The error is "Attempt to modify read-only object" and is somehow caused
by a string indexing bug.  Here's a patch:

2006-05-24  Kevin Rodgers  <ihs_4664@yahoo.com>

	* subr.el (read-passwd): Fix END argument to add-text-properties.


*** lisp/subr.el~	2006-04-22 06:42:24.750000000 -0600
--- lisp/subr.el	2006-05-24 09:56:54.555250000 -0600
***************
*** 1542,1548 ****
   	    (echo-keystrokes 0)
   	    (cursor-in-echo-area t)
   	    (message-log-max nil))
! 	(add-text-properties 0 (length prompt)
   			     minibuffer-prompt-properties prompt)
   	(while (progn (message "%s%s"
   			       prompt
--- 1542,1548 ----
   	    (echo-keystrokes 0)
   	    (cursor-in-echo-area t)
   	    (message-log-max nil))
! 	(add-text-properties 0 (1- (length prompt))
   			     minibuffer-prompt-properties prompt)
   	(while (progn (message "%s%s"
   			       prompt

Thanks,
-- 
Kevin

Re: [bug] read-passwd of CVS Emacs

[David Kastrup]

Kevin Rodgers <ihs_4664@yahoo.com> writes:

> Kazu Yamamoto (山本和彦) wrote:
>> If the CONFIRM argument is specified to read-passwd of CVS Emacs, it
>> causes an error after a user type a first password.
>>
>> 	(read-passwd "password: " t)
>
> The error is "Attempt to modify read-only object" and is somehow caused
> by a string indexing bug.  Here's a patch:
>
> 2006-05-24  Kevin Rodgers  <ihs_4664@yahoo.com>
>
> 	* subr.el (read-passwd): Fix END argument to add-text-properties.
>
>
> *** lisp/subr.el~	2006-04-22 06:42:24.750000000 -0600
> --- lisp/subr.el	2006-05-24 09:56:54.555250000 -0600
> ***************
> *** 1542,1548 ****
>   	    (echo-keystrokes 0)
>   	    (cursor-in-echo-area t)
>   	    (message-log-max nil))
> ! 	(add-text-properties 0 (length prompt)
>   			     minibuffer-prompt-properties prompt)
>   	(while (progn (message "%s%s"
>   			       prompt
> --- 1542,1548 ----
>   	    (echo-keystrokes 0)
>   	    (cursor-in-echo-area t)
>   	    (message-log-max nil))
> ! 	(add-text-properties 0 (1- (length prompt))
>   			     minibuffer-prompt-properties prompt)
>   	(while (progn (message "%s%s"
>   			       prompt

Looks like causing trouble when the prompt is an empty string.  And it
looks like it would leave the properties off the last character of the
prompt.  Correct?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [bug] read-passwd of CVS Emacs

[Kevin Rodgers]

David Kastrup wrote:
> Kevin Rodgers <ihs_4664@yahoo.com> writes:
> 
>> Kazu Yamamoto (山本和彦) wrote:
>>> If the CONFIRM argument is specified to read-passwd of CVS Emacs, it
>>> causes an error after a user type a first password.
>>>
>>> 	(read-passwd "password: " t)
>> The error is "Attempt to modify read-only object" and is somehow caused
>> by a string indexing bug.  Here's a patch:
...
> Looks like causing trouble when the prompt is an empty string.  And it
> looks like it would leave the properties off the last character of the
> prompt.  Correct?

You're right on both counts -- thanks for catching that!  I should have
investigated further before posting a patch that has nothing to do with
the underlying error.

The strange thing is that the error is definitely triggered by the call
to add-text-properties, but only in the second recursive call (not the
first):

Debugger entered--Lisp error: (error "Attempt to modify read-only object")
   add-text-properties(0 18 (read-only t face minibuffer-prompt) 
"Confirm password: ")
   byte-code(... [inhibit-quit confirm success prompt default second nil 
read-passwd "Confirm password: " arrayp clear-string message "Password 
not repeated accurately; please start over" sit-for 1 0 t 
add-text-properties "%s%s" make-string 46 read-char-exclusive 13 10 27 
clear-this-command-keys 21 "" 8 127 char-to-string -1 "" first 
message-log-max cursor-in-echo-area echo-keystrokes c pass 
minibuffer-prompt-properties new-char new-pass] 7)
   read-passwd("Confirm password: " nil nil)
   byte-code(... [inhibit-quit confirm success prompt default second nil 
read-passwd "Confirm password: " arrayp clear-string message "Password 
not repeated accurately; please start over" sit-for 1 0 t 
add-text-properties "%s%s" make-string 46 read-char-exclusive 13 10 27 
clear-this-command-keys 21 "" 8 127 char-to-string -1 "" first 
message-log-max cursor-in-echo-area echo-keystrokes c pass 
minibuffer-prompt-properties new-char new-pass] 7)
   read-passwd(#("password: " 0 10 (face minibuffer-prompt read-only t)) t)
   eval((read-passwd #("password: " 0 10 (face minibuffer-prompt 
read-only t)) t))
   eval-last-sexp-1(t)
   eval-last-sexp(t)
   eval-print-last-sexp()
   call-interactively(eval-print-last-sexp)

I checked the way minibuffer-prompt-properties are added in
src/minibuf.c and found that read_minibuf puts front-sticky,
rear-nonsticky, and field properties on the prompt first.  And emulating
that in read-passwd seems to do the trick:

2006-05-24  Kevin Rodgers  <ihs_4664@yahoo.com>
	* subr.el (read-passwd): Put front-sticky, rear-nonsticky, and field
	properties on PROMPT before adding minibuffer-prompt-properties, just
	like read_minibuf does.


*** lisp/subr.el~	2006-04-22 06:42:24.750000000 -0600
--- lisp/subr.el	2006-05-24 12:13:17.751556000 -0600
***************
*** 1541,1548 ****
   	    (c 0)
   	    (echo-keystrokes 0)
   	    (cursor-in-echo-area t)
! 	    (message-log-max nil))
! 	(add-text-properties 0 (length prompt)
   			     minibuffer-prompt-properties prompt)
   	(while (progn (message "%s%s"
   			       prompt
--- 1541,1552 ----
   	    (c 0)
   	    (echo-keystrokes 0)
   	    (cursor-in-echo-area t)
! 	    (message-log-max nil)
! 	    (prompt-length (length prompt)))
! 	(put-text-property 0 prompt-length 'front-sticky t prompt)
! 	(put-text-property 0 prompt-length 'rear-nonsticky t prompt)
! 	(put-text-property 0 prompt-length 'field t prompt)
! 	(add-text-properties 0 prompt-length
   			     minibuffer-prompt-properties prompt)
   	(while (progn (message "%s%s"
   			       prompt

Thanks,
-- 
Kevin

Re: [bug] read-passwd of CVS Emacs

[Johan Bockgård]

Kevin Rodgers <ihs_4664@yahoo.com> writes:

> David Kastrup wrote:
>> Kevin Rodgers <ihs_4664@yahoo.com> writes:
>> 
>>> Kazu Yamamoto (山本和彦) wrote:
>>>> If the CONFIRM argument is specified to read-passwd of CVS Emacs, it
>>>> causes an error after a user type a first password.
>>>>
>>>> 	(read-passwd "password: " t)
>>> The error is "Attempt to modify read-only object" and is somehow caused
>>> by a string indexing bug.  Here's a patch:
> ...
>> Looks like causing trouble when the prompt is an empty string. And
>> it looks like it would leave the properties off the last character
>> of the prompt. Correct?
>
> You're right on both counts -- thanks for catching that! I should
> have investigated further before posting a patch that has nothing to
> do with the underlying error.

Your latest patch also has nothing to do with it AFAICT. The string is
in pure storage--you need to make a copy.

-- 
Johan Bockgård

Re: [bug] read-passwd of CVS Emacs

[David Kastrup]

Kevin Rodgers <ihs_4664@yahoo.com> writes:

> David Kastrup wrote:
>> Kevin Rodgers <ihs_4664@yahoo.com> writes:
>>
>>> Kazu Yamamoto (山本和彦) wrote:
>>>> If the CONFIRM argument is specified to read-passwd of CVS Emacs, it
>>>> causes an error after a user type a first password.
>>>>
>>>> 	(read-passwd "password: " t)
>>> The error is "Attempt to modify read-only object" and is somehow caused
>>> by a string indexing bug.  Here's a patch:
> ...
>> Looks like causing trouble when the prompt is an empty string.  And it
>> looks like it would leave the properties off the last character of the
>> prompt.  Correct?
>
> You're right on both counts -- thanks for catching that!  I should have
> investigated further before posting a patch that has nothing to do with
> the underlying error.
>
> The strange thing is that the error is definitely triggered by the call
> to add-text-properties, but only in the second recursive call (not the
> first):

Maybe because adding text properties to an already read-only text is
prohibited?  In that case one would either have to refrain from doing
it, or bind inhibit-read-only to t while doing it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [bug] read-passwd of CVS Emacs

[Richard Stallman]

    The error is "Attempt to modify read-only object" and is somehow caused
    by a string indexing bug.  Here's a patch:

There is no indexing bug.  The bug is that it modifies the string
that was passed in by the caller.

This seems to fix it for me.  But I see that you proposed
another change, putting various other properties on the prompt.
Is that still necessary, when this is done?  I think it won't
be needed, because this prompt doesn't go in the minibuffer,
so you can't move point over it or insert other text after it.

Index: subr.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v
retrieving revision 1.511
diff -c -c -r1.511 subr.el
*** subr.el	24 May 2006 13:22:12 -0000	1.511
--- subr.el	24 May 2006 22:06:17 -0000
***************
*** 1637,1642 ****
--- 1637,1645 ----
  		(sit-for 1))))
  	  success)
        (let ((pass nil)
+ 	    ;; Copy it so that add-text-properties won't modify
+ 	    ;; the object that was passed in by the caller.
+ 	    (prompt (copy-sequence prompt))
  	    (c 0)
  	    (echo-keystrokes 0)
  	    (cursor-in-echo-area t)

Re: [bug] read-passwd of CVS Emacs

[Kevin Rodgers]

Richard Stallman wrote:
> There is no indexing bug.  The bug is that it modifies the string
> that was passed in by the caller.
 >
> This seems to fix it for me.  But I see that you proposed
> another change, putting various other properties on the prompt.
> Is that still necessary, when this is done?  I think it won't
> be needed, because this prompt doesn't go in the minibuffer,
> so you can't move point over it or insert other text after it.

Sorry for posting not 1 but 2 irrelevant patches.  I'm sure you're
right, that the front-sticky, rear-nonsticky, and field properties
are not needed since read-passwd displays the prompt in the echo
area instead of the minibuffer.

Thanks,
-- 
Kevin