bug#20211: 24.4; sendmail.el

bug#20211: 24.4; sendmail.el

[Richard Ryniker]

In file lisp/mail/sendmail.el, in function sendmail-send-it,
if variable mail-interactive has been
changed from its default value t to nil, no errbuf is created.
On the last line of the sendmail-send-it function, errbuf is
used as the argument for switch-to-buffer-other-window, which
faults because errbuf is not a buffer in this case.

Patch to use the appropriate actual buffer when errbuf does not exist:


[ryniker@puget ryniker]$ diff -c tmp/sendmail.el  sendmail.el 
*** tmp/sendmail.el	Thu Mar 26 20:42:09 2015
--- sendmail.el	Thu Mar 26 20:07:32 2015
***************
*** 1302,1308 ****
        (if (and (bufferp errbuf)
                 (not error))
            (kill-buffer errbuf)
!         (switch-to-buffer-other-window errbuf)))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")
  
--- 1302,1308 ----
        (if (and (bufferp errbuf)
                 (not error))
            (kill-buffer errbuf)
!         (switch-to-buffer-other-window (cond ((bufferp errbuf) errbuf) (t mailbuf)))))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")
  






In GNU Emacs 24.4.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.9)
 of 2015-03-25 on puget
Windowing system distributor `Fedora Project', version 11.0.11404000
Important settings:
  value of $LC_ALL: C
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix

Major mode: Mail

Minor modes in effect:
  shell-dirtrack-mode: t
  display-time-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  auto-fill-function: mail-mode-auto-fill
  transient-mark-mode: t
  abbrev-mode: t

Recent input:
g u n z i p SPC s e n d m <tab> <return> c p SPC <backspace> 
<backspace> <backspace> m k d i <backspace> <backspace> 
<backspace> <backspace> c o <backspace> <backspace> 
<f7> <down> <down> <down> <up> d <up> <up> d <up> <up> 
SPC c p SPC / u s r / l o c a l / s h a r e / e m a 
c <tab> ; o <tab> <backspace> <backspace> l i <tab> 
<backspace> i s <tab> <backspace> <backspace> <backspace> 
<tab> 2 <tab> l i <tab> s e n d m <tab> <backspace> 
<backspace> <backspace> <backspace> <backspace> m a 
i <tab> s e n d m a i l . <tab> t m p / <return> g 
u n z i p SPC t m p / s e n d m <tab> <return> d i 
f f SPC - c SPC s e n d m <backspace> <backspace> <backspace> 
<backspace> <backspace> t m p / s e n d m <tab> SPC 
s e n d m a i <tab> <return> <help-echo> <f12> <f11> 
<f6> <prior> M-x r e p o r t - e m a c <tab> <return> 
s e n d m a i l . e l <return> <up> <up> <up> <up> 
<up> <up> <up> <up> <up> <up> <up> <up> <down> <down> 
<end> <backspace> <backspace> <backspace> <backspace> 
m a i l / e m a <tab> <backspace> <backspace> <backspace> 
<backspace> <f5> m a i l <return> C-s e m a c <return> 
<f3> <home> C-k C-k C-c C-f C-f m a i l / e m a c <tab> 
<return> <help-echo> <next> C-x k <return> <f7> <down> 
<down> SPC m m e - g <down> M-x r e p o r t - e m a 
c s - <tab> <return>

Recent messages:
Checking 48 files in /usr/local/share/emacs/24.4/lisp/emulation...
Checking 151 files in /usr/local/share/emacs/24.4/lisp/emacs-lisp...
Checking 24 files in /usr/local/share/emacs/24.4/lisp/cedet...
Checking 57 files in /usr/local/share/emacs/24.4/lisp/calendar...
Checking 87 files in /usr/local/share/emacs/24.4/lisp/calc...
Checking 95 files in /usr/local/share/emacs/24.4/lisp/obsolete...
Checking for load-path shadows...done
Beginning of buffer [6 times]
Mark saved where search started
<<< Type SPC or RET to bury the buffer list >>>

Load-path shadows:
/home/ryniker/sendmail hides /usr/local/share/emacs/24.4/lisp/mail/sendmail

Features:
(dired-aux shadow emacsbug mule-util pp help-mode misearch multi-isearch
qp mailalias rmailmm message format-spec rfc822 mml easymenu mml-sec
mm-decode mm-bodies mm-encode mailabbrev gmm-utils mailheader mail-parse
rfc2231 shell pcomplete comint ansi-color ring dired time rmail sendmail
derived rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils
jka-compr ebuff-menu time-date tooltip electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
gfilenotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 100633 18476)
 (symbols 48 19099 0)
 (miscs 40 103 361)
 (strings 32 15687 3595)
 (string-bytes 1 442867)
 (vectors 16 11317)
 (vector-slots 8 407439 14817)
 (floats 8 83 832)
 (intervals 56 1627 522)
 (buffers 960 19)
 (heap 1024 48286 931))

bug#20211: 24.4; sendmail.el

[martin rudalics]

 > !         (switch-to-buffer-other-window (cond ((bufferp errbuf) errbuf) (t mailbuf)))))))

Do we really want to switch to a buffer when there was no error?  I'd
think we should proceed as follows: If `errbuf' exists then if `error'
is non-nil switch to `errbuf' in another window and kill it otherwise.
Do nothing if `errbuf' does not exist.  What do you think?

Thanks, martin

bug#20211: 24.4; sendmail.el

[Richard Ryniker]

Your strategy is better.  I focused too closely on simple avoidance of
the fault.  In fact, I believe the test you want is already present in
the previous two lines.  Tberefore, I think this does what you suggest:


*** 1302,1308 ****
        (if (and (bufferp errbuf)
                 (not error))
            (kill-buffer errbuf)
!         (switch-to-buffer-other-window errbuf)))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")
  
--- 1305,1312 ----
        (if (and (bufferp errbuf)
                 (not error))
            (kill-buffer errbuf)
!         (if (bufferp errbuf)
! 	    (switch-to-buffer-other-window errbuf))))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")

bug#20211: 24.4; sendmail.el

[martin rudalics]

 >          (if (and (bufferp errbuf)
 >                   (not error))
 >              (kill-buffer errbuf)
 > !         (if (bufferp errbuf)
 > ! 	    (switch-to-buffer-other-window errbuf))))))

Still slightly too complicated.  We should be able to use the form

(bufferp errbuf)

once only, preferably starting with

(when (bufferp errbuf)
   ...

and avoid the "not" ;-)

martin

bug#20211: 24.4; sendmail.el

[Richard Ryniker]

> We should be able to use the form (bufferp errbuf) once only

I concur.  Does the following look sensible to you?  Should we endeavor
to confirm good behavior when mail-interactive is true and the
sendmail program fails?

*** 1299,1308 ****
                    (error "Sending...failed to %s"
                           (buffer-substring (point-min) (point-max)))))))
        (kill-buffer tembuf)
!       (if (and (bufferp errbuf)
!                (not error))
!           (kill-buffer errbuf)
!         (switch-to-buffer-other-window errbuf)))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")
  
--- 1302,1310 ----
                    (error "Sending...failed to %s"
                           (buffer-substring (point-min) (point-max)))))))
        (kill-buffer tembuf)
!       (when (bufferp errbuf)
!         (if error (switch-to-buffer-other-window errbuf)
!           (kill-buffer errbuf))))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")

bug#20211: 24.4; sendmail.el

[martin rudalics]

 > Does the following look sensible to you?

Yes, thanks.  Two minor nitpicks: I'd put a newline after

(if error

and use `buffer-live-p' instead of `bufferp'.  Though it's not likely
someone might have killed errbuf in between and we want to avoid a
confusing error message in that case.  And please provide a suitable
ChangeLog entry.

 > Should we endeavor
 > to confirm good behavior when mail-interactive is true and the
 > sendmail program fails?

I'm not sure I understand you here.  IIUC "when mail-interactive is true
and the sendmail program fails" we show the error.  Don't we?

martin

bug#20211: 24.4; sendmail.el

[Richard Ryniker]

*** emacs-24.4/lisp/mail/sendmail.el	Fri Mar 21 01:34:40 2014
--- /home/ryniker/sendmail.el	Sun Mar 29 17:14:53 2015
***************
*** 1299,1308 ****
                    (error "Sending...failed to %s"
                           (buffer-substring (point-min) (point-max)))))))
        (kill-buffer tembuf)
!       (if (and (bufferp errbuf)
!                (not error))
!           (kill-buffer errbuf)
!         (switch-to-buffer-other-window errbuf)))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")
  
--- 1299,1308 ----
                    (error "Sending...failed to %s"
                           (buffer-substring (point-min) (point-max)))))))
        (kill-buffer tembuf)
!       (when (buffer-live-p errbuf)
!         (if error
!             (switch-to-buffer-other-window errbuf)
!           (kill-buffer errbuf))))))
  
  (autoload 'rmail-output-to-rmail-buffer "rmailout")


Proposed emacs-24.4/lisp/Changelog entry:

2015-03-29  Richard Ryniker  <ryniker@alum.mit.edu>

	* mail/sendmail.el (sendmail-send-it): Do not attempt to switch to
	non-existent buffer.  (errbuf is not created when customization
	variable mail-interactive is nil.)


>IIUC "when mail-interactive is true and the sendmail program fails" we
>show the error.  Don't we?

I do not know.  I believe this patch only changes the sendmail.el logic
in the mail-interactive true case to use buffer-live-p instead of
bufferp.  The error I encountered occurs only when mail-interactive is
nil: in this case, errbuf is never created and the attempt to switch to
the non-existant buffer causes a fault.

What errors a Mail User Agent reports synchronously, and what errors are
reported asynchronously via mail from the Mail Transfer Agent, must
depend on the programs actually used.  In my case (using exim) all errors
I have encountered (unknown recipient, malformed address, etc.) seem to
be reported asynchronously by the MTA.  To directly test
mail-intereactive true, I should have to concoct some test sendmail
program that delivers errors as desired.  Not difficult; I can do this if
you think it useful.

bug#20211: 24.4; sendmail.el

[martin rudalics]

 > Proposed emacs-24.4/lisp/Changelog entry:

Even Emacs 24.5 is frozen now so I installed it in Emacs 25.1 as

    d16fb74..d9eff02  master -> master

Please have a look.  If we decide to release 24.6 we can backport it
there.

 >> IIUC "when mail-interactive is true and the sendmail program fails" we
 >> show the error.  Don't we?
 >
 > I do not know.  I believe this patch only changes the sendmail.el logic
 > in the mail-interactive true case to use buffer-live-p instead of
 > bufferp.  The error I encountered occurs only when mail-interactive is
 > nil: in this case, errbuf is never created and the attempt to switch to
 > the non-existant buffer causes a fault.

Yes but due to your patch we can switch to errbuf only if it is live so
the error should not happen any more.

Thanks, martin