bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[Noam Postavsky <npostavs@gmail.com>]

Matthew Newton <matt@knosis.org> writes:

> (defun desktop--v2s (value)
>   ...
>   (cond
>    ((byte-code-function-p value)
>     (let* ((pass1 (mapcar #'desktop--v2s value))
>            (special (assq nil pass1)))
>       (if special
>           (cons nil `(make-byte-code
>                       ,@(mapcar (lambda (el)
>                                   (if (eq (car el) 'must)
>                                       `',(cdr el) (cdr el)))
>                                 pass1)))
>         (cons 'may `[,@(mapcar #'cdr pass1)]))))

I don't think `[...] will ever return a byte code function, so the
non-special case doesn't seem quite right.

> Are there security concerns or other considerations?

Well, as (info "(elisp) Byte-Code Objects") mentions, passing the wrong
args to make-byte-code can produce a function which will crash Emacs
when called, so it's somewhat high risk.

>> The "unprintable entity" comes from desktop--v2s, looks like it doesn't
>> handle compiled function values, so that's why :print-func and
>> :insert-func get messed up like that.

I note that the functions in question come from this code in kmacro.el:

(defun kmacro-to-register (r)
  ... (registerv-make
		   last-kbd-macro
		   :jump-func 'kmacro-execute-from-register
		   :print-func (lambda (k)
				 (princ (format "a keyboard macro:\n   %s"
						(format-kbd-macro k))))
		   :insert-func (lambda (k)
				  (insert (format-kbd-macro k))))

While in register.el we have:

(cl-defun registerv-make (data &key print-func jump-func insert-func)
  ...
  (declare (obsolete "Use your own type with methods on register-val-(insert|describe|jump-to)" "27.1"))

So perhaps this is a good opportunity to change the kmacro register
stuff to use the non-obsolete way.
 
>>> I’ve also seen “Unprintable entity” show up when a buffer position
>>> register points to a nonexistent buffer. Not sure how to reproduce
>>> that one.
>> 
>> I guess if you save a position in a buffer, then kill the buffer.
>> 
> Here is what I found: if the killed buffer visits a file, it gets
> converted into a file-query. If there is no `buffer-file-name` then it
> stays a file marker pointing to nowhere. Desktop handles both of those
> cases correctly. So while I have seen it happen I’m not sure of the
> case where a buffer position register becomes an “Unprintable
> entity”. Did you find reproduction steps?

Ah, no, I was just guessing based on your initial description, no idea
how to reproduce it if desktop.el is already handling these cases.