GNU bug report logs - #1102
23.0.60; add-hook should check that its function arg is a function.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 1102 in the body.
You can then email your comments to 1102 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Phil Hagelberg <phil <at> hagelb.org>
To: emacs-pretest-bug <at> gnu.org
Subject: 23.0.60; add-hook should check that its function arg is a function.
Date: Mon, 06 Oct 2008 09:59:57 -0700

add-hook may be called with a function argument that is not a function:

(add-hook 'emacs-lisp-mode-hook 22)

This is functionally a no-op since run-mode-hooks will ignore such
values, but it would probably be better to get an error when add-hook
is called.

Here's a patch. I'm not sure what the convention is for how these
types of error messages should read, so that could be changed.

diff --git a/lisp/subr.el b/lisp/subr.el
index d62b38b..49e9470 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1132,6 +1132,7 @@ HOOK is void, it is first set to nil.  If HOOK's value is a single
 function, it is changed to a list of functions."
   (or (boundp hook) (set hook nil))
   (or (default-boundp hook) (set-default hook nil))
+  (unless (functionp function) (error "`function' argument was not a function"))
   (if local (unless (local-variable-if-set-p hook)
 	      (set (make-local-variable hook) (list t)))
     ;; Detect the case where make-local-variable was used on a hook



In GNU Emacs 23.0.60.1 (i686-pc-linux-gnu, GTK+ Version 2.12.9)
 of 2008-09-29 on pdp10
Windowing system distributor `The X.Org Foundation', version 11.0.10400090
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default-enable-multibyte-characters: t

Major mode: Group

Minor modes in effect:
  shell-dirtrack-mode: t
  jabber-activity-mode: t
  gnus-topic-mode: t
  gnus-undo-mode: t
  show-paren-mode: t
  recentf-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
o k SPC ' g n u s - a r t i c l e - m o d e - h o o 
k C-S-h v <return> q SPC ' l o n g l i n e - <backspace> 
s - m o d e ) C-S-x C-S-s C-S-x C-S-e C-S-x k <return> 
C-S-x C-S-o SPC SPC SPC SPC M-x l o n g l i n e s - 
- <backspace> m o d e <return> M-x M-p <return> M-< 
C-S-x o C-S-p C-S-x o C-S-p <return> C-S-n <return> 
C-S-x o C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n 
C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n 
C-S-n C-S-n C-S-n C-S-n C-S-n C-S-v C-S-v C-S-v SPC 
SPC SPC SPC SPC C-S-x C-S-o C-S-n <return> C-S-n <return> 
C-S-n <return> C-S-n <return> C-S-n <return> C-S-n 
<return> C-S-n <return> C-S-n <return> C-S-n <return> 
C-S-n <return> C-S-n <return> C-S-n C-S-n <return> 
C-S-n <return> C-S-n <return> C-S-n <return> q g p 
<return> q C-S-n <return> k k k k k k q p n <return> 
d q g p <return> q p p <return> M-> C-S-p C-S-p C-S-p 
<return> C-S-n <down-mouse-1> <mouse-1> <help-echo> 
<down-mouse-1> <mouse-1> SPC C-S-x o C-S-x o M-u C-S-p 
<return> q g <down-mouse-1> <mouse-1> <down-mouse-1> 
<mouse-1> g p p p p <return> M-> p C-S-x o M-> M-< 
C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n 
C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n 
C-S-n C-S-n C-S-n C-S-n C-S-n C-S-n t t C-S-p C-S-p 
C-S-p <return> <help-echo> <down-mouse-1> <mouse-movement> 
<help-echo> <mouse-movement> <mouse-movement> <help-echo> 
<mouse-movement> <help-echo> <mouse-movement> <mouse-movement> 
<help-echo> <mouse-movement> <help-echo> <drag-mouse-1> 
<help-echo> <help-echo> <down-mouse-1> <mouse-1> C-S-x 
o C-S-x o ! C-S-p C-S-p <return> M-u <help-echo> q 
g M-x r e p o r t - e m a c s - b u g <return>

Recent messages:
Generating summary...done
Mark set [3 times]
No more articles [2 times]
Reading active file via nnnil...done
Reading active file via nnml...
nnml: Reading incoming mail from file...
nnml: Reading incoming mail (no new mail)...done
Reading active file via nnml...done
Reading active file from archive via nnfolder...done
Checking new news...done

Message #10 received at 1102 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Phil Hagelberg <phil <at> hagelb.org>
To: 1102 <at> debbugs.gnu.org
Subject: Modification of patch
Date: Mon, 06 Oct 2008 11:24:22 -0700

Changed the patch from the initial bug report.

add-hook should allow symbols since it's reasonable to refer to
functions that haven't been defined yet.

-Phil

diff --git a/lisp/subr.el b/lisp/subr.el
index d62b38b..d315c27 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1132,6 +1132,8 @@ HOOK is void, it is first set to nil.  If HOOK's value is a single
 function, it is changed to a list of functions."
   (or (boundp hook) (set hook nil))
   (or (default-boundp hook) (set-default hook nil))
+  (unless (or (functionp function) (symbolp function))
+              (error "`function' argument was not a function"))
   (if local (unless (local-variable-if-set-p hook)
 	      (set (make-local-variable hook) (list t)))
     ;; Detect the case where make-local-variable was used on a hook

Message #15 received at 1102 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Phil Hagelberg <phil <at> hagelb.org>
Cc: 1102 <at> debbugs.gnu.org
Subject: Re: bug#1102: Modification of patch
Date: Thu, 16 Oct 2008 01:54:49 -0400

Phil Hagelberg wrote:

> add-hook should allow symbols since it's reasonable to refer to
> functions that haven't been defined yet.

Doesn't it therefore seem a bit pointless to check? There will always
be ways to shoot oneself in the foot. I imagine many functions don't
check the type of their arguments.

Message #20 received at 1102 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Phil Hagelberg <phil <at> hagelb.org>
To: Glenn Morris <rgm <at> gnu.org>
Subject: Re: bug#1102: Modification of patch
Date: Thu, 16 Oct 2008 09:09:32 -0700

Glenn Morris <rgm <at> gnu.org> writes:

>> add-hook should allow symbols since it's reasonable to refer to
>> functions that haven't been defined yet.
>
> Doesn't it therefore seem a bit pointless to check? There will always
> be ways to shoot oneself in the foot. I imagine many functions don't
> check the type of their arguments.

Yeah, you're probably right; in retrospect I'm not sure why it's
necessary. Feel free to close as a wontfix.

-Phil

Message #27 received at 1102 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Phil Hagelberg <phil <at> hagelb.org>
Cc: 1102 <at> debbugs.gnu.org, control <at> debbugs.gnu.org
Subject: Re: bug#1102: Modification of patch
Date: Thu, 16 Oct 2008 21:28:08 -0400

tag 1102 +wontfix
thanks

> Yeah, you're probably right; in retrospect I'm not sure why it's
> necessary. Feel free to close as a wontfix.

Thank you,


        Stefan

Message #34 received at 1102-done <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 1102-done <at> debbugs.gnu.org
Subject: Re: bug#1102: add-hook should check that its function arg is a
	function
Date: Sat, 09 Jul 2011 14:46:34 -0400

I don't see a need to keep open this particular report, which was marked
"wontfix" some time ago.

Phil Hagelberg wrote:

> Yeah, you're probably right; in retrospect I'm not sure why it's
> necessary. Feel free to close as a wontfix.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.