Re: [PATCH] * src/eval.c: Stop checking for nvars, and use only CONSP

[Naoya Yamashita]

Thanks, eli.

> They aren't redundant, they were added to avoid crashes in certain
> rare use cases.  See commit 93511e9 and the emacs-devel discussion
> which the log message refers to.  If the problems described there are
> no longer pertinent, can you explain why?  TIA.

Thanks for mentioning 93511e9.

I compared the two and found that the problem of crashing on
(if . "abc") crash problem was fixed in the former.

However

    (let ((vars (list 'v))))
      (setcdr vars vars)
      (eval (list let vars))))

and

    (let ((clauses (list '((progn (setcdr clauses "ouch")) nil)))))
      (eval (cons 'cond clauses))))

did not crash.  The circular list example became an infinite loop
in an older version of b3a3ed5 (even older changes to eval.c).
My change removed the list length checker, which is vulnerable to
list changes.  Furthermore, Emacs with my patch passes these test
cases.

My patched Emacs does not segfault the Pip example either.

    (let ((cons-cell '((a 2) (b 3))))
      (eval `(let ((x (setcdr ',cons-cell nil))
               . ,cons-cell)
           (message "foo"))))
    ;;=> foo

Please let me know if there are any test cases I'm missing.

> P.S. When I see code in Emacs that looks wrong/redundant/unclear, I
> find it useful to look at the VC history of that code.  This is how I
> discovered where this particular "redundancy" came from, just 3.5
> years ago.

Thanks, I find `M-x vc-history-region` and this is very useful!
I should use before write a patch.