allout patch - more safe-local-variables, plus autoloads

allout patch - more safe-local-variables, plus autoloads

[Ken Manheimer]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

in this patch a bunch more allout variables are attributed as
'safe-local-variable, and i've added the autoload cookie so the
attributions are included in loaddefs.  the corresponding ChangeLog
entry is also attached.
--
ken manheimer
ken.manheimer@gmail.com
http://myriadicity.net

[-- Attachment #2: allout-patch.txt --]
[-- Type: text/plain, Size: 7191 bytes --]

Index: allout.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/allout.el,v
retrieving revision 1.71
diff -u -u -r1.71 allout.el
--- allout.el	13 Apr 2006 10:44:37 -0000	1.71
+++ allout.el	17 Apr 2006 19:02:54 -0000
@@ -84,8 +84,6 @@
                           (require 'pgg-gpg)
                           (require 'overlay)
 			  ))
-(autoload 'pgg-gpg-symmetric-key-p "pgg-gpg"
-  "True if decoded armor MESSAGE-KEYS has symmetric encryption indicator.")
 
 ;;;_* USER CUSTOMIZATION VARIABLES:
 
@@ -199,6 +197,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-show-bodies)
+;;;###autoload
+(put 'allout-show-bodies 'safe-local-variable t)
 
 ;;;_  = allout-header-prefix
 (defcustom allout-header-prefix "."
@@ -212,6 +212,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-header-prefix)
+;;;###autoload
+(put 'allout-header-prefix 'safe-local-variable t)
 ;;;_  = allout-primary-bullet
 (defcustom allout-primary-bullet "*"
   "Bullet used for top-level outline topics.
@@ -227,6 +229,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-primary-bullet)
+;;;###autoload
+(put 'allout-primary-bullet 'safe-local-variable t)
 ;;;_  = allout-plain-bullets-string
 (defcustom allout-plain-bullets-string ".,"
   "*The bullets normally used in outline topic prefixes.
@@ -241,6 +245,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-plain-bullets-string)
+;;;###autoload
+(put 'allout-plain-bullets-string 'safe-local-variable t)
 ;;;_  = allout-distinctive-bullets-string
 (defcustom allout-distinctive-bullets-string "*+-=>()[{}&!?#%\"X@$~_\\:;^"
   "*Persistent outline header bullets used to distinguish special topics.
@@ -274,6 +280,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-distinctive-bullets-string)
+;;;###autoload
+(put 'allout-distinctive-bullets-string 'safe-local-variable t)
 
 ;;;_  = allout-use-mode-specific-leader
 (defcustom allout-use-mode-specific-leader t
@@ -305,6 +313,8 @@
 		 (const allout-mode-leaders)
 		 (const comment-start))
   :group 'allout)
+;;;###autoload
+(put 'allout-use-mode-specific-leader 'safe-local-variable t)
 ;;;_  = allout-mode-leaders
 (defvar allout-mode-leaders '()
   "Specific allout-prefix leading strings per major modes.
@@ -330,6 +340,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-old-style-prefixes)
+;;;###autoload
+(put 'allout-old-style-prefixes 'safe-local-variable t)
 ;;;_  = allout-stylish-prefixes - alternating bullets
 (defcustom allout-stylish-prefixes t
   "*Do fancy stuff with topic prefix bullets according to level, etc.
@@ -376,6 +388,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-stylish-prefixes)
+;;;###autoload
+(put 'allout-stylish-prefixes 'safe-local-variable t)
 
 ;;;_  = allout-numbered-bullet
 (defcustom allout-numbered-bullet "#"
@@ -388,6 +402,8 @@
   :type '(choice (const nil) string)
   :group 'allout)
 (make-variable-buffer-local 'allout-numbered-bullet)
+;;;###autoload
+(put 'allout-numbered-bullet 'safe-local-variable t)
 ;;;_  = allout-file-xref-bullet
 (defcustom allout-file-xref-bullet "@"
   "*Bullet signifying file cross-references, for `allout-resolve-xref'.
@@ -395,6 +411,8 @@
 Set this var to the bullet you want to use for file cross-references."
   :type '(choice (const nil) string)
   :group 'allout)
+;;;###autoload
+(put 'allout-file-xref-bullet 'safe-local-variable t)
 ;;;_  = allout-presentation-padding
 (defcustom allout-presentation-padding 2
   "*Presentation-format white-space padding factor, for greater indent."
@@ -402,6 +420,8 @@
   :group 'allout)
 
 (make-variable-buffer-local 'allout-presentation-padding)
+;;;###autoload
+(put 'allout-presentation-padding 'safe-local-variable t)
 
 ;;;_  = allout-abbreviate-flattened-numbering
 (defcustom allout-abbreviate-flattened-numbering nil
@@ -455,11 +475,16 @@
   :group 'allout)
 
 ;;;_ + Topic encryption
+;;;_  = allout-encryption group
+(defgroup allout-encryption nil
+  "Settings for topic encryption features of allout outliner."
+  :group 'allout)
 ;;;_  = allout-topic-encryption-bullet
 (defcustom allout-topic-encryption-bullet "~"
   "*Bullet signifying encryption of the entry's body."
   :type '(choice (const nil) string)
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 ;;;_  = allout-passphrase-verifier-handling
 (defcustom allout-passphrase-verifier-handling t
   "*Enable use of symmetric encryption passphrase verifier if non-nil.
@@ -467,7 +492,8 @@
 See the docstring for the `allout-enable-file-variable-adjustment'
 variable for details about allout ajustment of file variables."
   :type 'boolean
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-passphrase-verifier-handling)
 ;;;_  = allout-passphrase-hint-handling
 (defcustom allout-passphrase-hint-handling 'always
@@ -482,7 +508,8 @@
   :type '(choice (const always)
                  (const needed)
                  (const disabled))
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-passphrase-hint-handling)
 ;;;_  = allout-encrypt-unencrypted-on-saves
 (defcustom allout-encrypt-unencrypted-on-saves t
@@ -514,7 +541,8 @@
   :type '(choice (const :tag "Yes" t)
                  (const :tag "All except current topic" except-current)
                  (const :tag "No" nil))
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-encrypt-unencrypted-on-saves)
 
 ;;;_ + Miscellaneous customization
@@ -585,6 +613,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-use-hanging-indents)
+;;;###autoload
+(put 'allout-use-hanging-indents 'safe-local-variable t)
 
 ;;;_  = allout-reindent-bodies
 (defcustom allout-reindent-bodies (if allout-use-hanging-indents
@@ -602,6 +632,8 @@
   :group 'allout)
 
 (make-variable-buffer-local 'allout-reindent-bodies)
+;;;###autoload
+(put 'allout-reindent-bodies 'safe-local-variable t)
 
 ;;;_  = allout-enable-file-variable-adjustment
 (defcustom allout-enable-file-variable-adjustment t
@@ -1053,6 +1085,7 @@
 the emacs buffer state, if file variable adjustments are enabled.  See
 `allout-enable-file-variable-adjustment' for details about that.")
 (make-variable-buffer-local 'allout-passphrase-verifier-string)
+;;;###autoload
 (put 'allout-passphrase-verifier-string 'safe-local-variable t)
 ;;;_   = allout-passphrase-hint-string
 (defvar allout-passphrase-hint-string ""
@@ -1065,8 +1098,9 @@
 state, if file variable adjustments are enabled.  See
 `allout-enable-file-variable-adjustment' for details about that.")
 (make-variable-buffer-local 'allout-passphrase-hint-string)
-(put 'allout-passphrase-hint-string 'safe-local-variable t)
 (setq-default allout-passphrase-hint-string "")
+;;;###autoload
+(put 'allout-passphrase-hint-string 'safe-local-variable t)
 ;;;_   = allout-after-save-decrypt
 (defvar allout-after-save-decrypt nil
   "Internal variable, is nil or has the value of two points:



[-- Attachment #3: allout-ChangeLog.txt --]
[-- Type: text/plain, Size: 585 bytes --]

2006-04-17  Ken Manheimer  <ken.manheimer@gmail.com>

	* allout.el: Remove local autoload declaration for
	pgg-gpg-symmetric-key-p, since that's now done in pgg-gpg.el.

	(allout-show-bodies, allout-header-prefix, allout-primary-bullet,
	allout-plain-bullets-string, allout-distinctive-bullets-string,
	allout-use-mode-specific-leader, allout-old-style-prefixes,
	allout-stylish-prefixes, allout-numbered-bullet,
	allout-file-xref-bullet, allout-presentation-padding,
	allout-use-hanging-indents, allout-reindent-bodies): Mark as
	safe-local-variable and add cookie for autoloading.



[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: allout patch - more safe-local-variables, plus autoloads

[Richard Stallman]

I see that you've used t as the property value.
That allows any type of value.

Most of these variables are supposed to be strings.
I don't know what would happen if they were set to other types;
did you check each one?

It would be safer to use stringp as the property value
for a variable that is supposed to be a string.

Re: allout patch - more safe-local-variables, plus autoloads

[Stefan Monnier]

> I see that you've used t as the property value.
> That allows any type of value.

> Most of these variables are supposed to be strings.
> I don't know what would happen if they were set to other types;
> did you check each one?

> It would be safer to use stringp as the property value
> for a variable that is supposed to be a string.

BTW, I thought we had decided not to support the special case t to mean "any
value is safe" and instead force the use of explicit predicates (which can
still be (lambda (x) t), of course: you can make it harder for people to
shoot themselves in the foot, but you can't prevent them from doing so if
they're sufficiently motivated).

Shouldn't we do that (basically the patch below, plus many other patches to
fix all the places where we use t).


        Stefan


--- files.el	13 avr 2006 10:22:03 -0400	1.826
+++ files.el	18 avr 2006 09:10:34 -0400	
@@ -2643,15 +2643,11 @@
  * There is a matching entry (SYM . VAL) in the
    `safe-local-variable-values' user option.
 
- * The `safe-local-variable' property of SYM is t.
-
  * The `safe-local-variable' property of SYM is a function that
    evaluates to a non-nil value with VAL as an argument."
   (or (member (cons sym val) safe-local-variable-values)
       (let ((safep (get sym 'safe-local-variable)))
-	(or (eq safep t)
-	    (and (functionp safep)
-		 (funcall safep val))))))
+        (and (functionp safep) (funcall safep val)))))
 
 (defun risky-local-variable-p (sym &optional ignored)
   "Non-nil if SYM could be dangerous as a file-local variable.

Re: allout patch - more safe-local-variables, plus autoloads

[Richard Stallman]

I think it is legitimate to use t for boolean variables.  All values
are safe for them.

If we don't use t, what would we use for them?

Re: allout patch - more safe-local-variables, plus autoloads

[Stefan Monnier]

> I think it is legitimate to use t for boolean variables.  All values
> are safe for them.

> If we don't use t, what would we use for them?

If we disallow t and want a quick&dirty replacement predicate for boolean
values, we can either introduce booleanp (seems like a good predicate to
have around anyway) or just use symbolp.


        Stefan

Re: allout patch - more safe-local-variables, plus autoloads

[Richard Stallman]

    If we disallow t and want a quick&dirty replacement predicate for boolean
    values, we can either introduce booleanp (seems like a good predicate to
    have around anyway) or just use symbolp.

symbolp is not correct; a boolean variable can have any Lisp object
as its value.  If booleanp is mean to mean "is t or nil", that would
also be incorrect.

I don't see any great need to eliminate t as a value,
and certainly not at the cost of any real annoyance.

Re: allout patch - more safe-local-variables, plus autoloads

[Ken Manheimer]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On 4/18/06, Richard Stallman <rms@gnu.org> wrote:
> I see that you've used t as the property value.
> That allows any type of value.
>
> Most of these variables are supposed to be strings.
> I don't know what would happen if they were set to other types;
> did you check each one?
>
> It would be safer to use stringp as the property value
> for a variable that is supposed to be a string.

i was too casual when i read the info about safe-local-variable.  i've
revised my patch to better discriminate values, where appropriate. 
the revised patch and the ChangeLog are attached.
--
ken manheimer
ken.manheimer@gmail.com
http://myriadicity.net

[-- Attachment #2: allout-patch.txt --]
[-- Type: text/plain, Size: 7516 bytes --]

Index: allout.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/allout.el,v
retrieving revision 1.71
diff -u -u -r1.71 allout.el
--- allout.el	13 Apr 2006 10:44:37 -0000	1.71
+++ allout.el	19 Apr 2006 23:02:28 -0000
@@ -84,8 +84,6 @@
                           (require 'pgg-gpg)
                           (require 'overlay)
 			  ))
-(autoload 'pgg-gpg-symmetric-key-p "pgg-gpg"
-  "True if decoded armor MESSAGE-KEYS has symmetric encryption indicator.")
 
 ;;;_* USER CUSTOMIZATION VARIABLES:
 
@@ -199,6 +197,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-show-bodies)
+;;;###autoload
+(put 'allout-show-bodies 'safe-local-variable t)
 
 ;;;_  = allout-header-prefix
 (defcustom allout-header-prefix "."
@@ -212,6 +212,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-header-prefix)
+;;;###autoload
+(put 'allout-header-prefix 'safe-local-variable 'stringp)
 ;;;_  = allout-primary-bullet
 (defcustom allout-primary-bullet "*"
   "Bullet used for top-level outline topics.
@@ -227,6 +229,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-primary-bullet)
+;;;###autoload
+(put 'allout-primary-bullet 'safe-local-variable 'stringp)
 ;;;_  = allout-plain-bullets-string
 (defcustom allout-plain-bullets-string ".,"
   "*The bullets normally used in outline topic prefixes.
@@ -241,6 +245,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-plain-bullets-string)
+;;;###autoload
+(put 'allout-plain-bullets-string 'safe-local-variable 'stringp)
 ;;;_  = allout-distinctive-bullets-string
 (defcustom allout-distinctive-bullets-string "*+-=>()[{}&!?#%\"X@$~_\\:;^"
   "*Persistent outline header bullets used to distinguish special topics.
@@ -274,6 +280,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-distinctive-bullets-string)
+;;;###autoload
+(put 'allout-distinctive-bullets-string 'safe-local-variable 'stringp)
 
 ;;;_  = allout-use-mode-specific-leader
 (defcustom allout-use-mode-specific-leader t
@@ -305,6 +313,8 @@
 		 (const allout-mode-leaders)
 		 (const comment-start))
   :group 'allout)
+;;;###autoload
+(put 'allout-use-mode-specific-leader 'safe-local-variable t)
 ;;;_  = allout-mode-leaders
 (defvar allout-mode-leaders '()
   "Specific allout-prefix leading strings per major modes.
@@ -330,6 +340,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-old-style-prefixes)
+;;;###autoload
+(put 'allout-old-style-prefixes 'safe-local-variable t)
 ;;;_  = allout-stylish-prefixes - alternating bullets
 (defcustom allout-stylish-prefixes t
   "*Do fancy stuff with topic prefix bullets according to level, etc.
@@ -376,6 +388,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-stylish-prefixes)
+;;;###autoload
+(put 'allout-stylish-prefixes 'safe-local-variable t)
 
 ;;;_  = allout-numbered-bullet
 (defcustom allout-numbered-bullet "#"
@@ -388,6 +402,9 @@
   :type '(choice (const nil) string)
   :group 'allout)
 (make-variable-buffer-local 'allout-numbered-bullet)
+;;;###autoload
+(put 'allout-numbered-bullet 'safe-local-variable
+     (lambda (x) (or (not x) (stringp x))))
 ;;;_  = allout-file-xref-bullet
 (defcustom allout-file-xref-bullet "@"
   "*Bullet signifying file cross-references, for `allout-resolve-xref'.
@@ -395,6 +412,9 @@
 Set this var to the bullet you want to use for file cross-references."
   :type '(choice (const nil) string)
   :group 'allout)
+;;;###autoload
+(put 'allout-file-xref-bullet 'safe-local-variable
+     (lambda (x) (or (not x) (stringp x))))
 ;;;_  = allout-presentation-padding
 (defcustom allout-presentation-padding 2
   "*Presentation-format white-space padding factor, for greater indent."
@@ -402,6 +422,8 @@
   :group 'allout)
 
 (make-variable-buffer-local 'allout-presentation-padding)
+;;;###autoload
+(put 'allout-presentation-padding 'safe-local-variable 'integerp)
 
 ;;;_  = allout-abbreviate-flattened-numbering
 (defcustom allout-abbreviate-flattened-numbering nil
@@ -455,11 +477,16 @@
   :group 'allout)
 
 ;;;_ + Topic encryption
+;;;_  = allout-encryption group
+(defgroup allout-encryption nil
+  "Settings for topic encryption features of allout outliner."
+  :group 'allout)
 ;;;_  = allout-topic-encryption-bullet
 (defcustom allout-topic-encryption-bullet "~"
   "*Bullet signifying encryption of the entry's body."
   :type '(choice (const nil) string)
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 ;;;_  = allout-passphrase-verifier-handling
 (defcustom allout-passphrase-verifier-handling t
   "*Enable use of symmetric encryption passphrase verifier if non-nil.
@@ -467,7 +494,8 @@
 See the docstring for the `allout-enable-file-variable-adjustment'
 variable for details about allout ajustment of file variables."
   :type 'boolean
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-passphrase-verifier-handling)
 ;;;_  = allout-passphrase-hint-handling
 (defcustom allout-passphrase-hint-handling 'always
@@ -482,7 +510,8 @@
   :type '(choice (const always)
                  (const needed)
                  (const disabled))
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-passphrase-hint-handling)
 ;;;_  = allout-encrypt-unencrypted-on-saves
 (defcustom allout-encrypt-unencrypted-on-saves t
@@ -514,7 +543,8 @@
   :type '(choice (const :tag "Yes" t)
                  (const :tag "All except current topic" except-current)
                  (const :tag "No" nil))
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-encrypt-unencrypted-on-saves)
 
 ;;;_ + Miscellaneous customization
@@ -585,6 +615,8 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-use-hanging-indents)
+;;;###autoload
+(put 'allout-use-hanging-indents 'safe-local-variable t)
 
 ;;;_  = allout-reindent-bodies
 (defcustom allout-reindent-bodies (if allout-use-hanging-indents
@@ -602,6 +634,9 @@
   :group 'allout)
 
 (make-variable-buffer-local 'allout-reindent-bodies)
+;;;###autoload
+(put 'allout-reindent-bodies 'safe-local-variable
+     (lambda (x) (member x '(nil t text force))))
 
 ;;;_  = allout-enable-file-variable-adjustment
 (defcustom allout-enable-file-variable-adjustment t
@@ -1053,7 +1088,8 @@
 the emacs buffer state, if file variable adjustments are enabled.  See
 `allout-enable-file-variable-adjustment' for details about that.")
 (make-variable-buffer-local 'allout-passphrase-verifier-string)
-(put 'allout-passphrase-verifier-string 'safe-local-variable t)
+;;;###autoload
+(put 'allout-passphrase-verifier-string 'safe-local-variable 'stringp)
 ;;;_   = allout-passphrase-hint-string
 (defvar allout-passphrase-hint-string ""
   "Variable used to retain reminder string for file's encryption passphrase.
@@ -1065,8 +1101,9 @@
 state, if file variable adjustments are enabled.  See
 `allout-enable-file-variable-adjustment' for details about that.")
 (make-variable-buffer-local 'allout-passphrase-hint-string)
-(put 'allout-passphrase-hint-string 'safe-local-variable t)
 (setq-default allout-passphrase-hint-string "")
+;;;###autoload
+(put 'allout-passphrase-hint-string 'safe-local-variable 'stringp)
 ;;;_   = allout-after-save-decrypt
 (defvar allout-after-save-decrypt nil
   "Internal variable, is nil or has the value of two points:



[-- Attachment #3: allout-ChangeLog.txt --]
[-- Type: text/plain, Size: 626 bytes --]

2006-04-17  Ken Manheimer  <ken.manheimer@gmail.com>

	* allout.el: Remove local autoload declaration for
	pgg-gpg-symmetric-key-p, since that's now done in pgg-gpg.el.

	(allout-show-bodies, allout-header-prefix, allout-primary-bullet,
	allout-plain-bullets-string, allout-distinctive-bullets-string,
	allout-use-mode-specific-leader, allout-old-style-prefixes,
	allout-stylish-prefixes, allout-numbered-bullet,
	allout-file-xref-bullet, allout-presentation-padding,
	allout-use-hanging-indents, allout-reindent-bodies): Mark as
	safe-local-variable with suitable value spec, and add autoload
	cookie for loaddefs inclusion.

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: allout patch - more safe-local-variables, plus autoloads

[Luc Teirlinck]

Richard Stallman wrote (in response to Stefan Monnier):

       If we disallow t and want a quick&dirty replacement predicate
       for boolean values, we can either introduce booleanp (seems
       like a good predicate to have around anyway) or just use symbolp.

   symbolp is not correct; a boolean variable can have any Lisp object
   as its value.  If booleanp is mean to mean "is t or nil", that
   would also be incorrect.

This is not about correctness, but about safety.  Any non-nil value
for a minor mode variable is "correct", in that it is equivalent with
t, but why would anybody actually _want_ to use any other value than t
in a local variables list.  If anybody actually did it, I would want
to see it because it would make me suspicious.

Currently, putting:

;;; no-byte-compile: (funcall (lambda () ... some hairy code ... ))

in a local variables list gets evaluated without asking, because
`no-byte-compile' has a safe-local-variable property of t.  The "hairy
code" is not tested for safety at all.  I checked this using obviously
unsafe code.

But if you got asked, would you really just immediately respond yes
without taking a look at that lambda expression, because that lambda
expression is equivalent with t anyway and hence perfectly "correct"?

If I really believed that the above code currently could be exploited
in harmful way, I would not be posting it publicly.  For it to do
harm, no-byte-compile should be evaluated twice as in
(eval no-byte-compile), although there are other ways to produce
repeated evaluation than using eval.  If somebody tried to get
(eval no-byte-compile) or similar evaluated elsewhere in the local
variables list, then the user would get asked about _that_.
(Otherwise this would be a serious bug).

But as we keep increasing the number of boolean variables with a
safe-local-variable property of t, then the probability increases that
by some freakish accident one of these boolean variables gets
evaluated twice in the Emacs source code.  This would be a bug,
because the boolean variable can take any value and hence should not
be evaluated twice.  But in practice, one always uses t or nil as
values, so the bug could easily escape detection for a long time
(unlike similar bugs for variables that are normally lists, where the
double evaluation would immediately throw an error, exposing the bug).

The above scenario might not seem excessively likely but why take _any_
risk, since there is no good reason to use non-nil values other than t
for boolean variables in a local variables list?

I do believe that for variables of type DEFVAR_BOOL all values are
completely safe, since they get immediately translated to nil or t anyway.

Sincerely,

Luc.

Re: allout patch - more safe-local-variables, plus autoloads

[Richard Stallman]

Ok, please install.

Re: allout patch - more safe-local-variables, plus autoloads

[Stefan Monnier]

>     If we disallow t and want a quick&dirty replacement predicate for boolean
>     values, we can either introduce booleanp (seems like a good predicate to
>     have around anyway) or just use symbolp.

> symbolp is not correct; a boolean variable can have any Lisp object
> as its value.  If booleanp is mean to mean "is t or nil", that would
> also be incorrect.

The idea behind safe-local-variable, AFAIK, is to accept values which are
known to be safe.  It may reject valid values if they're not known to be
safe and that's OK because the user still gets a chance to accept
the value.

In the case of boolean variables, history shows that they have
a non-negligible chance of being extended in the future to give special
meaning to some other values (typically some other symbol, or to values
which are functions, ...).  So it's better to only consider as safe the
values nil and t which are least likely to change meaning (and become
potentially unsafe) in the future.

> I don't see any great need to eliminate t as a value,
> and certainly not at the cost of any real annoyance.

The intention is to force the author to think about which predicate to use,
in the hope it will reduce the number of cases where the safe-local-variable
predicate ends up accepting unsafe values as well.


        Stefan

autoloads for safe-local-variables (was: allout patch - more safe-local-variables, plus autoloads)

[Reiner Steib]

On Thu, Apr 20 2006, Ken Manheimer wrote:

> +;;;###autoload
> +(put 'allout-show-bodies 'safe-local-variable t)

In my commits, I've put in the same line...

;;;###autoload(put 'page-delimiter 'safe-local-variable t)

... like it is done e.g. in `code-pages.el':

;;;###autoload(autoload-coding-system 'cp437 '(require 'code-pages))

Which style is preferable?

(info "(elisp)Autoload") didn't clarify my question.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: autoloads for safe-local-variables (was: allout patch - moresafe-local-variables, plus autoloads)

[Stuart D. Herring]

>> +;;;###autoload
>> +(put 'allout-show-bodies 'safe-local-variable t)
>
> In my commits, I've put in the same line...
>
> ;;;###autoload(put 'page-delimiter 'safe-local-variable t)
>
> ... like it is done e.g. in `code-pages.el':
>
> ;;;###autoload(autoload-coding-system 'cp437 '(require 'code-pages))
>
> Which style is preferable?

The first style leaves the code as active code, executed when the file is
loaded.  The second style generates no code in file.elc but only in
loaddefs.el.  So if it's logically part of the library, you want the first
form; if it's some sort of reference to the library, but not really part
of it, like the code-pages example, you want the second one.  For a simple
(put ...) it probably doesn't matter too much, but it probably would
interact better with, say, unload-feature to use the first one.

This gets me thinking, though -- does unload-feature remove plists from
symbols it's dumping?  If so, shouldn't it not remove the
`risky-local-variable' property if any?  Apologies if this is just
confusion.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: allout patch - more safe-local-variables, plus autoloads

[Richard Stallman]

    In the case of boolean variables, history shows that they have
    a non-negligible chance of being extended in the future to give special
    meaning to some other values (typically some other symbol, or to values
    which are functions, ...).

OK, I take the point.

Would you like to make the change?

Re: autoloads for safe-local-variables (was: allout patch - more safe-local-variables, plus autoloads)

[Richard Stallman]

    > +;;;###autoload
    > +(put 'allout-show-bodies 'safe-local-variable t)

    In my commits, I've put in the same line...

    ;;;###autoload(put 'page-delimiter 'safe-local-variable t)

The first one executes the code at startup
and again when the file is loaded.

The second one executes the code at startup
and does not execute it when the file is loaded.
(It is in a comment, after all.)

For these properties, I see no reason to execute it
again when the file is loaded, so the second form seems better.

Re: allout patch - more safe-local-variables, plus autoloads

[Ken Manheimer]

in many cases, the qualifying criteria is already present, in the form
of the custom-type information:

On 4/20/06, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >     If we disallow t and want a quick&dirty replacement predicate for boolean
> >     values, we can either introduce booleanp (seems like a good predicate to
> >     have around anyway) or just use symbolp.
> [...]
> > I don't see any great need to eliminate t as a value,
> > and certainly not at the cost of any real annoyance.
>
> The intention is to force the author to think about which predicate to use,
> in the hope it will reduce the number of cases where the safe-local-variable
> predicate ends up accepting unsafe values as well.

seems to me we're heading down the road of duplicating the mechanisms
of the custom type system, and that we should be using it.  i strongly
suspect that the custom types of customizable variables are
appropriate as safety criteria in many cases, as well.  we wouldn't
want to assume that, but i suspect it would be very useful to make it
easy for authors to use the custom-type criteria for this purpose.

i'm thinking something along the lines of:

(defun qualify-value-by-custom-type (symbol value)
  "True if SYMBOL's type constraint is satisfied by VALUE."
  (let ((custom-type (get symbol 'custom-type))
        type)
    (when custom-type
      (setq type (widget-convert custom-type))
      (widget-apply type :match value))))

and have that function used when the `safe-local-variable' property
value is `t'.  the author can still specify alternate criteria if the
custom-type criteria is not suitable, or if the variable is state
rather than a customized.

this scheme would work just right for my allout safe-local-variables -
14 are customizable, where the custom-type criteria are just right for
the safety spec, and 3 are state vars, where i need to designate the
criteria.  when i do need to designate the criteria, i would like to
be able to use the expressiveness of the custom type system.  (it
already provides a boolean criterion, and everything else you could
need, like composition, etc.)

the one catch is that the customizable variables would have to be
autoloaded to have the custom information to test against - that seems
like the right thing to do, but would require code changes.

what do you think?
--
ken manheimer
ken.manheimer@gmail.com
http://myriadicity.net

Re: allout patch - more safe-local-variables, plus autoloads

[David Kastrup]

"Ken Manheimer" <ken.manheimer@gmail.com> writes:

> in many cases, the qualifying criteria is already present, in the form
> of the custom-type information:
>
> seems to me we're heading down the road of duplicating the mechanisms
> of the custom type system, and that we should be using it.  i strongly
> suspect that the custom types of customizable variables are
> appropriate as safety criteria in many cases, as well.  we wouldn't
> want to assume that, but i suspect it would be very useful to make it
> easy for authors to use the custom-type criteria for this purpose.

I don't see that.  If a variable can be dangerous, I think it very
unlikely that all uses conforming to its customization type would be
harmless.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: allout patch - more safe-local-variables, plus autoloads

[Ken Manheimer]

On 4/21/06, David Kastrup <dak@gnu.org> wrote:

> "Ken Manheimer" <ken.manheimer@gmail.com> writes:
>
> > in many cases, the qualifying criteria is already present, in the form
> > of the custom-type information:
> >
> > seems to me we're heading down the road of duplicating the mechanisms
> > of the custom type system, and that we should be using it.  i strongly
> > suspect that the custom types of customizable variables are
> > appropriate as safety criteria in many cases, as well.  we wouldn't
> > want to assume that, but i suspect it would be very useful to make it
> > easy for authors to use the custom-type criteria for this purpose.
>
> I don't see that.  If a variable can be dangerous, I think it very
> unlikely that all uses conforming to its customization type would be
> harmless.

why not?  seems to me that the customization criteria are in
substantial part about safety - a good author is preventing their
users from shooting themselves in the foot with problematic custom
values.  the only difference is that less attention is likely to be
paid to users who are deviously *trying* to blow their foot off.-)  (i
wonder how often current safety tests exceed their variable's
customization tests, when present?  i suspect not many.)

the crucial thing is that the safety criteria should never *conflict*
with the customization criteria, and in fact, should always obtain for
it.  it may be that people need to be thinking more stringently about
their customization criteria, but the safety concerns apply there as
much as anywhere.

i could be wrong here - counter examples welcome.  for what it's
worth, it happens to work quite nicely for allout's variables.
--
ken manheimer
ken.manheimer@gmail.com
http://myriadicity.net

Re: allout patch - more safe-local-variables, plus autoloads

[Stefan Monnier]

> seems to me we're heading down the road of duplicating the mechanisms
> of the custom type system, and that we should be using it.

If you check this mailing-list, you'll see that it all started with Richard
suggesting exactly that and we ended up with `safe-local-variable'.

Now, this doesn't mean it was a bad idea.  Just that you can't rely solely
on the custom type mostly because it may include unsafe values.

But, yes, maybe we could add a special predicate that basically says
"consider as safe anything that the custom type accepts".  The only problem
I can see with it is that the custom-type is typically not preloaded (even
if you place a ;;;###autoload in front of the defcustom), so we'd either
arrange for it to be preloaded or to force the corresponding package to be
autoloaded whenever the variable appears in a "Local Variables" section.


        Stefan

Re: allout patch - more safe-local-variables, plus autoloads

[Ken Manheimer]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

this is yet another version of a patch (and ChangeLog) for allout.el
which provides appropriate safe-local-variable settings.  this patch
differs from the prior ones in two ways:

  - it does not use `t' for the safe-local-variables property value,
in anticipation
    the this feature may become obsolete
  - rectifies a bug in backward-compat move-{beginning,end}-of-line functions
    for emacs versions that lack them

this should supercede any other versions of the patch, and hopefully
will obviate the need for further patches if/when the
safe-local-variables value policy changes.

--
ken manheimer
ken.manheimer@gmail.com
http://myriadicity.net

[-- Attachment #2: allout-patch.txt --]
[-- Type: text/plain, Size: 11612 bytes --]

Index: allout.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/allout.el,v
retrieving revision 1.71
diff -u -u -r1.71 allout.el
--- allout.el	13 Apr 2006 10:44:37 -0000	1.71
+++ allout.el	24 Apr 2006 19:55:04 -0000
@@ -84,8 +84,6 @@
                           (require 'pgg-gpg)
                           (require 'overlay)
 			  ))
-(autoload 'pgg-gpg-symmetric-key-p "pgg-gpg"
-  "True if decoded armor MESSAGE-KEYS has symmetric encryption indicator.")
 
 ;;;_* USER CUSTOMIZATION VARIABLES:
 
@@ -199,6 +197,9 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-show-bodies)
+;;;###autoload
+(put 'allout-show-bodies 'safe-local-variable
+     (lambda (x) (member x '(t nil))))
 
 ;;;_  = allout-header-prefix
 (defcustom allout-header-prefix "."
@@ -212,6 +213,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-header-prefix)
+;;;###autoload
+(put 'allout-header-prefix 'safe-local-variable 'stringp)
 ;;;_  = allout-primary-bullet
 (defcustom allout-primary-bullet "*"
   "Bullet used for top-level outline topics.
@@ -227,6 +230,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-primary-bullet)
+;;;###autoload
+(put 'allout-primary-bullet 'safe-local-variable 'stringp)
 ;;;_  = allout-plain-bullets-string
 (defcustom allout-plain-bullets-string ".,"
   "*The bullets normally used in outline topic prefixes.
@@ -241,6 +246,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-plain-bullets-string)
+;;;###autoload
+(put 'allout-plain-bullets-string 'safe-local-variable 'stringp)
 ;;;_  = allout-distinctive-bullets-string
 (defcustom allout-distinctive-bullets-string "*+-=>()[{}&!?#%\"X@$~_\\:;^"
   "*Persistent outline header bullets used to distinguish special topics.
@@ -274,6 +281,8 @@
   :type 'string
   :group 'allout)
 (make-variable-buffer-local 'allout-distinctive-bullets-string)
+;;;###autoload
+(put 'allout-distinctive-bullets-string 'safe-local-variable 'stringp)
 
 ;;;_  = allout-use-mode-specific-leader
 (defcustom allout-use-mode-specific-leader t
@@ -292,7 +301,7 @@
 Set to the symbol for either of `allout-mode-leaders' or
 `comment-start' to use only one of them, respectively.
 
-Value nil means to always use the default \(`.').
+Value nil means to always use the default \(`.').t
 
 comment-start strings that do not end in spaces are tripled, and an
 `_' underscore is tacked on the end, to distinguish them from regular
@@ -305,6 +314,9 @@
 		 (const allout-mode-leaders)
 		 (const comment-start))
   :group 'allout)
+;;;###autoload
+(put 'allout-use-mode-specific-leader 'safe-local-variable
+     (lambda (x) (or (member x '(t nil)) (stringp x))))
 ;;;_  = allout-mode-leaders
 (defvar allout-mode-leaders '()
   "Specific allout-prefix leading strings per major modes.
@@ -330,6 +342,9 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-old-style-prefixes)
+;;;###autoload
+(put 'allout-old-style-prefixes 'safe-local-variable
+     (lambda (x) (member x '(t nil))))
 ;;;_  = allout-stylish-prefixes - alternating bullets
 (defcustom allout-stylish-prefixes t
   "*Do fancy stuff with topic prefix bullets according to level, etc.
@@ -376,6 +391,9 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-stylish-prefixes)
+;;;###autoload
+(put 'allout-stylish-prefixes 'safe-local-variable
+     (lambda (x) (member x '(t nil))))
 
 ;;;_  = allout-numbered-bullet
 (defcustom allout-numbered-bullet "#"
@@ -388,6 +406,9 @@
   :type '(choice (const nil) string)
   :group 'allout)
 (make-variable-buffer-local 'allout-numbered-bullet)
+;;;###autoload
+(put 'allout-numbered-bullet 'safe-local-variable
+     (lambda (x) (or (not x) (stringp x))))
 ;;;_  = allout-file-xref-bullet
 (defcustom allout-file-xref-bullet "@"
   "*Bullet signifying file cross-references, for `allout-resolve-xref'.
@@ -395,6 +416,9 @@
 Set this var to the bullet you want to use for file cross-references."
   :type '(choice (const nil) string)
   :group 'allout)
+;;;###autoload
+(put 'allout-file-xref-bullet 'safe-local-variable
+     (lambda (x) (or (not x) (stringp x))))
 ;;;_  = allout-presentation-padding
 (defcustom allout-presentation-padding 2
   "*Presentation-format white-space padding factor, for greater indent."
@@ -402,6 +426,8 @@
   :group 'allout)
 
 (make-variable-buffer-local 'allout-presentation-padding)
+;;;###autoload
+(put 'allout-presentation-padding 'safe-local-variable 'integerp)
 
 ;;;_  = allout-abbreviate-flattened-numbering
 (defcustom allout-abbreviate-flattened-numbering nil
@@ -455,11 +481,16 @@
   :group 'allout)
 
 ;;;_ + Topic encryption
+;;;_  = allout-encryption group
+(defgroup allout-encryption nil
+  "Settings for topic encryption features of allout outliner."
+  :group 'allout)
 ;;;_  = allout-topic-encryption-bullet
 (defcustom allout-topic-encryption-bullet "~"
   "*Bullet signifying encryption of the entry's body."
   :type '(choice (const nil) string)
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 ;;;_  = allout-passphrase-verifier-handling
 (defcustom allout-passphrase-verifier-handling t
   "*Enable use of symmetric encryption passphrase verifier if non-nil.
@@ -467,7 +498,8 @@
 See the docstring for the `allout-enable-file-variable-adjustment'
 variable for details about allout ajustment of file variables."
   :type 'boolean
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-passphrase-verifier-handling)
 ;;;_  = allout-passphrase-hint-handling
 (defcustom allout-passphrase-hint-handling 'always
@@ -482,7 +514,8 @@
   :type '(choice (const always)
                  (const needed)
                  (const disabled))
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-passphrase-hint-handling)
 ;;;_  = allout-encrypt-unencrypted-on-saves
 (defcustom allout-encrypt-unencrypted-on-saves t
@@ -514,7 +547,8 @@
   :type '(choice (const :tag "Yes" t)
                  (const :tag "All except current topic" except-current)
                  (const :tag "No" nil))
-  :group 'allout)
+  :version "22.0"
+  :group 'allout-encryption)
 (make-variable-buffer-local 'allout-encrypt-unencrypted-on-saves)
 
 ;;;_ + Miscellaneous customization
@@ -585,6 +619,9 @@
   :type 'boolean
   :group 'allout)
 (make-variable-buffer-local 'allout-use-hanging-indents)
+;;;###autoload
+(put 'allout-use-hanging-indents 'safe-local-variable
+     (lambda (x) (member x '(t nil))))
 
 ;;;_  = allout-reindent-bodies
 (defcustom allout-reindent-bodies (if allout-use-hanging-indents
@@ -602,6 +639,9 @@
   :group 'allout)
 
 (make-variable-buffer-local 'allout-reindent-bodies)
+;;;###autoload
+(put 'allout-reindent-bodies 'safe-local-variable
+     (lambda (x) (member x '(nil t text force))))
 
 ;;;_  = allout-enable-file-variable-adjustment
 (defcustom allout-enable-file-variable-adjustment t
@@ -667,7 +707,9 @@
 case the value of `allout-default-layout' is used.")
 (make-variable-buffer-local 'allout-layout)
 ;;;###autoload
-(put 'allout-layout 'safe-local-variable t)
+(put 'allout-layout 'safe-local-variable
+     (lambda (x) (or (numberp x) (listp x) (integerp x)
+                     (member x '(: * + -)))))
 
 ;;;_  : Topic header format
 ;;;_   = allout-regexp
@@ -1053,7 +1095,8 @@
 the emacs buffer state, if file variable adjustments are enabled.  See
 `allout-enable-file-variable-adjustment' for details about that.")
 (make-variable-buffer-local 'allout-passphrase-verifier-string)
-(put 'allout-passphrase-verifier-string 'safe-local-variable t)
+;;;###autoload
+(put 'allout-passphrase-verifier-string 'safe-local-variable 'stringp)
 ;;;_   = allout-passphrase-hint-string
 (defvar allout-passphrase-hint-string ""
   "Variable used to retain reminder string for file's encryption passphrase.
@@ -1065,8 +1108,9 @@
 state, if file variable adjustments are enabled.  See
 `allout-enable-file-variable-adjustment' for details about that.")
 (make-variable-buffer-local 'allout-passphrase-hint-string)
-(put 'allout-passphrase-hint-string 'safe-local-variable t)
 (setq-default allout-passphrase-hint-string "")
+;;;###autoload
+(put 'allout-passphrase-hint-string 'safe-local-variable 'stringp)
 ;;;_   = allout-after-save-decrypt
 (defvar allout-after-save-decrypt nil
   "Internal variable, is nil or has the value of two points:
@@ -1573,6 +1617,12 @@
 				       ; epoch, minor-mode key bindings:
 	   (setq allout-mode-map
 		 (produce-allout-mode-map allout-keybindings-list))
+           (substitute-key-definition 'beginning-of-line
+                                      'move-beginning-of-line
+                                      allout-mode-map global-map)
+           (substitute-key-definition 'end-of-line
+                                      'move-end-of-line
+                                      allout-mode-map global-map)
 	   (produce-allout-mode-menubar-entries)
 	   (fset 'allout-mode-map allout-mode-map)
 				       ; Include on minor-mode-map-alist,
@@ -5778,17 +5828,14 @@
       (if (/= arg 1)
           (condition-case nil (line-move (1- arg)) (error nil)))
 
-      (let ((orig (point)))
-        ;; Move to beginning-of-line, ignoring fields and invisibles.
-        (skip-chars-backward "^\n")
-        (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
-          (goto-char (if (featurep 'xemacs)
-                         (previous-property-change (point))
-                       (previous-char-property-change (point))))
-          (skip-chars-backward "^\n"))
-        (vertical-motion 0)
-        (if (/= orig (point))
-            (goto-char orig))))
+      ;; Move to beginning-of-line, ignoring fields and invisibles.
+      (skip-chars-backward "^\n")
+      (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
+        (goto-char (if (featurep 'xemacs)
+                       (previous-property-change (point))
+                     (previous-char-property-change (point))))
+        (skip-chars-backward "^\n"))
+      (vertical-motion 0))
 )
 ;;;_  > move-end-of-line if necessary - older emacs, xemacs
 (if (not (fboundp 'move-end-of-line))
@@ -5802,8 +5849,7 @@
 To ignore intangibility, bind `inhibit-point-motion-hooks' to t."
       (interactive "p")
       (or arg (setq arg 1))
-      (let ((orig (point))
-            done)
+      (let (done)
         (while (not done)
           (let ((newpos
                  (save-excursion
@@ -5813,8 +5859,10 @@
                             (error nil))
                           (not (bobp))
                           (progn
-                            (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
-                              (goto-char (previous-char-property-change (point))))
+                            (while (and (not (bobp))
+                                        (line-move-invisible-p (1- (point))))
+                              (goto-char
+                               (previous-char-property-change (point))))
                             (backward-char 1)))
                      (point)))))
             (goto-char newpos)
@@ -5827,9 +5875,7 @@
                   ;; and now we're not really at eol,
                   ;; keep going.
                   (setq arg 1)
-                (setq done t)))))
-        (if (/= orig (point))
-            (goto-char orig))))
+                (setq done t)))))))
   )
 ;;;_  > line-move-invisible-p if necessary
 (if (not (fboundp 'line-move-invisible-p))


[-- Attachment #3: allout-ChangeLog.txt --]
[-- Type: text/plain, Size: 914 bytes --]

2006-04-17  Ken Manheimer  <ken.manheimer@gmail.com>

	* allout.el: Remove local autoload declaration for
	pgg-gpg-symmetric-key-p, since that's now done in pgg-gpg.el.

	(allout-show-bodies, allout-header-prefix, allout-primary-bullet,
	allout-plain-bullets-string, allout-distinctive-bullets-string,
	allout-use-mode-specific-leader, allout-old-style-prefixes,
	allout-stylish-prefixes, allout-numbered-bullet,
	allout-file-xref-bullet, allout-presentation-padding,
	allout-use-hanging-indents, allout-reindent-bodies): Mark as
	safe-local-variable with suitable value spec, and add autoload
	cookie for loaddefs inclusion.  We use an explicit spec everywhere,
	rather than the defaulting 't', in anticipation of change in or
	obsolescence of the meaning of `t' in this context.

	(move-beginning-of-line, move-end-of-line): Repair so these compat
	functions now actually resituate the point, when appropriate.


[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel