bug#21602: 25.0.50; coding system seg fault

bug#21602: 25.0.50; coding system seg fault

[Richard Stallman]

When I try to send the following buffer with Mail mode,
I get a seg fault within write_region as it tries to handle
'us-ascii' as the mime charset.

If I delete the Content-Type header line, there is no problem.


From: Richard Stallman <rms@gnu.org>
To: hal@csail.mit.edu, Aubrey Colter <ajcolter@mit.edu>
Subject: Please don't promote Amazon!
Reply-to: rms@gnu.org
Date: Wed, 30 Sep 2015 22:23:49 -0400
X-Received-From: 2001:4830:134:3::e
Content-Type: text/plain; charset="us-ascii"
--text follows this line--
Please don't use Amazon gift cards as a reward.
That promotes a company that does nasty things to its customers,
its employees, and nearly everyone else it deals with.

------- Start of forwarded message -------
From: Aubrey Colter <ajcolter@mit.edu>
Date: Tue, 29 Sep 2015 12:27:20 -0400
To: csail-related@lists.csail.mit.edu
Subject: [csail-related] Seeking Usability Study Participants for MIT App
	Inventor!

The MIT App Inventor team is evaluating the MIT App Inventor user interface
through usability testing. The project is being lead by Aubrey Colter for
her Master of Engineering thesis.

We are inviting novice and hobbyist programmers who have little or no
experience with MIT App Inventor to participate in a one-on-one session to
evaluate the application. The MIT IS&T Usability Team will be conducting
these 30 to 45 minute evaluations at MIT (Building W92) or remotely via
WebEx, based on your preference.

If you are interested in participating in this evaluation, please complete
the brief survey found at https://goo.gl/UBasVm. The survey asks some
general questions to determine your eligibility to participate in the
study.

Once you have completed the survey, Aubrey Colter or a member of the
Usability Team will contact you to confirm your participation and provide
further details.

Usability testing participants will be entered into a drawing to receive
one of 15 $10 Amazon gift cards as a thank you for your participation in
this evaluation.

Thank you,
Aubrey Colter
MIT App Inventor Team
_______________________________________________
Csail-related mailing list
Csail-related@lists.csail.mit.edu
https://lists.csail.mit.edu/mailman/listinfo/csail-related
https://projects.csail.mit.edu/cgi-bin/wiki/view/CsailRelated/WebHome
------- End of forwarded message -------

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






In GNU Emacs 25.0.50.1 (i686-pc-linux-gnu, GTK+ Version 2.24.23)
 of 2015-08-12 on freetop
Repository revision: 79a169684dfad2c0bbb9fdbae539c1f30d9f0ac3
System Description:	Trisquel GNU/Linux 7.0, Belenos

Configured using:
 `configure 'CFLAGS=-g -O0''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GCONF GSETTINGS NOTIFY
LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK2 X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Mail

Minor modes in effect:
  shell-dirtrack-mode: t
  gpm-mouse-mode: t
  tooltip-mode: t
  global-eldoc-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent messages:
Mark set [3 times]
C-c C-g is undefined
Sending...done
Sending...
Wrote /home/rms/outgoing/out-136
Sending...done
Undo! [2 times]
Mark set [2 times]
File /home/rms/foo1 already exists; overwrite anyway? (y or n) y
Wrote /home/rms/foo1
Quit

Load-path shadows:
None found.

Features:
(shadow emacsbug mule-diag mailcap wid-edit jka-compr grep compile
shell pcomplete comint ansi-color epa-mail rect dired-aux etags
thingatpt xref project ring dabbrev sgml-mode rmailkwd epa derived epg
url-util url-parse auth-source cl-seq eieio byte-opt bytecomp
byte-compile cl-extra seq cconv eieio-core cl-macs gv gnus-util
password-cache url-vars shr dom subr-x browse-url qp mailalias
misearch multi-isearch rmailsum rmailout rmailmm message sendmail
format-spec rfc822 mml mml-sec mm-decode mm-bodies mm-encode
mailabbrev gmm-utils mailheader mail-parse rfc2231 rmail rfc2047
rfc2045 ietf-drums mm-util help-fns help-mode cl-loaddefs pcase cl-lib
mail-prsvr mail-utils dired t-mouse view time-date paren cus-start
cus-load advice finder-inf package easymenu epg-config mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet
lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak
czech european ethiopic indian cyrillic chinese charscript case-table
epa-hook jka-cmpr-hook help simple abbrev minibuffer cl-preloaded
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 dbusbind gfilenotify
dynamic-setting system-font-setting font-render-setting move-toolbar
gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 8 279423 61018)
 (symbols 24 25213 15)
 (miscs 20 1806 3185)
 (strings 16 38673 16011)
 (string-bytes 1 1512693)
 (vectors 8 25793)
 (vector-slots 4 1433588 41156)
 (floats 8 292 510)
 (intervals 28 47705 446)
 (buffers 520 159)
 (heap 1024 15828 1217))
[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]


-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Thu, 01 Oct 2015 22:32:02 -0400
> 
> 
> When I try to send the following buffer with Mail mode,
> I get a seg fault within write_region as it tries to handle
> 'us-ascii' as the mime charset.

I tried to reproduce this, but it didn't fail for me.  There are too
many parameters involved in sending email, starting with your
mail-client package, so it's small wonder I couldn't reproduce this
easily.

Could you please show a backtrace from the segfault, including the
variable(s) that caused it?  That might provide enough info to find
and fix this.

Thanks.

bug#21602: 25.0.50; coding system seg fault

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > When I try to send the following buffer with Mail mode,
  > > I get a seg fault within write_region as it tries to handle
  > > 'us-ascii' as the mime charset.

  > I tried to reproduce this, but it didn't fail for me.  There are too
  > many parameters involved in sending email, starting with your
  > mail-client package, so it's small wonder I couldn't reproduce this
  > easily.

I use Mail mode.  But the problem depends on my local mail setup.

I can reproduce the failure in emacs -Q with the code below.  The code
in my-send-1 is responsible for passing specifying the coding system,
but even if that value is wrong, write-region should not crash.


(setq mail-user-agent 'sendmail-user-agent)

  (setq send-mail-function 'my-send-it)

(defvar private-regexp
  "ffofoofofofofofo@")

(defun my-send-it ()
  (save-excursion
    (let (buffer-undo-list
	  (case-fold-search t)
	  list-to-undo)
      (if (save-excursion
	    (goto-char (point-min))
	    (re-search-forward "^to:")
	    (let ((end (save-excursion (forward-line 1) (point))))
	      (re-search-forward private-regexp end t)))
	  (save-excursion
	    (goto-char (point-min))
	    (re-search-forward "^reply-to:")
	    (let ((end (save-excursion (forward-line 1) (point))))
	      (replace-string "rms@" "rms-private@" nil (point) end)))
	(save-excursion
	  (goto-char (point-min))
	  (unless (re-search-forward "^bcc: rms-outgoing@gnu.org\n" nil t)
	    (re-search-forward "^reply-to:")
	    (beginning-of-line)
	    (insert "bcc: rms-outgoing@gnu.org\n"))
	  (setq list-to-undo buffer-undo-list)))
      (if (save-excursion
	    (goto-char (point-min))
	    (re-search-forward "^reply-to:")
	    (let ((end (save-excursion (forward-line 1) (point))))
	      (re-search-forward "rms-private" end t)))
	  (my-send-1 "out-")
	(my-send-1 "out-"))

      (while list-to-undo
	(let ((undo-in-progress t))
	  (setq list-to-undo (primitive-undo 1 list-to-undo)))))))

(defun my-send-1 (base)
  (let* ((default-directory (expand-file-name "~/outgoing/"))
	 (files (file-name-all-completions base default-directory))
	 (versions (mapcar (function (lambda (name)
				       (string-to-int (substring name (length base)))))
			   files))
	 (coding-system-for-write
	  (save-excursion
	    (goto-char (point-min))
	    (if (re-search-forward "^Content-Type:.*; *charset=\\([^;\n]+\\)"
				   nil t)
		(intern (downcase (match-string 1)))
	      coding-system-for-write))))
    (write-region (point-min) (point-max)
		  (format "%s%d" base
			  (if versions
			      (1+ (apply 'max versions))
			    1)))))


-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

bug#21602: 25.0.50; coding system seg fault

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

C backtrace followed by Lisp backtrace

#0  0x0816c2af in AREF (array=0, idx=2) at lisp.h:1474
#1  0x080f9cf9 in coding_inherit_eol_type (coding_system=1216824, parent=0)
    at coding.c:6005
#2  0x081b53b5 in choose_write_coding_system (start=6, end=22818, 
    filename=140507116, append=0, visit=0, lockname=140507116, 
    coding=0xbfffdef8) at fileio.c:4619
#3  0x081b58ab in write_region (start=6, end=22818, filename=140507116, 
    append=0, visit=0, lockname=140507116, mustbenew=0, desc=-1)
    at fileio.c:4793
#4  0x081b5473 in Fwrite_region (start=6, end=22818, filename=140507148, 
    append=0, visit=0, lockname=0, mustbenew=0) at fileio.c:4667
#5  0x081fee43 in eval_sub (form=143801747) at eval.c:2213
#6  0x081fb916 in Fprogn (body=143801915) at eval.c:453
#7  0x081fc4e2 in FletX (args=143801739) at eval.c:904
#8  0x081fea38 in eval_sub (form=143803323) at eval.c:2139
#9  0x081fb916 in Fprogn (body=143801955) at eval.c:453
#10 0x08200846 in funcall_lambda (fun=143801987, nargs=1, 
    arg_vector=0xbfffe3f0) at eval.c:2952
#11 0x082003bb in apply_lambda (fun=143801987, args=143815411, count=19)
    at eval.c:2834
#12 0x081ff0a1 in eval_sub (form=143815403) at eval.c:2264
#13 0x081fb916 in Fprogn (body=143815419) at eval.c:453
#14 0x081fb85b in Fif (args=143815371) at eval.c:405
#15 0x081fea38 in eval_sub (form=143815163) at eval.c:2139
#16 0x081fb916 in Fprogn (body=143815427) at eval.c:453
#17 0x081fc81e in Flet (args=143803179) at eval.c:974
#18 0x081fea38 in eval_sub (form=143803187) at eval.c:2139
#19 0x081fb916 in Fprogn (body=143803203) at eval.c:453
#20 0x081ee94d in Fsave_excursion (args=143803203) at editfns.c:1018
#21 0x081fea38 in eval_sub (form=143803195) at eval.c:2139
#22 0x081fb916 in Fprogn (body=143803227) at eval.c:453
#23 0x08200846 in funcall_lambda (fun=143803211, nargs=0, 
    arg_vector=0xbfffea08) at eval.c:2952
#24 0x0820019e in Ffuncall (nargs=1, args=0xbfffea04) at eval.c:2787
#25 0x0823d229 in exec_byte_code (bytestr=140534860, vector=143349373, 
    maxdepth=34, args_template=0, nargs=0, args=0x0) at bytecode.c:919
#26 0x082008e3 in funcall_lambda (fun=143196117, nargs=0, arg_vector=0x88b567d)
    at eval.c:2959
#27 0x082000d6 in Ffuncall (nargs=1, args=0xbfffef28) at eval.c:2775
#28 0x081f8c44 in Ffuncall_interactively (nargs=1, args=0xbfffef28)
    at callint.c:252
#29 0x081ffd72 in Ffuncall (nargs=2, args=0xbfffef24) at eval.c:2706
#30 0x081ff195 in Fapply (nargs=3, args=0xbfffef24) at eval.c:2297
#31 0x081f8fea in Fcall_interactively (function=744440, record_flag=0, 
    keys=139677285) at callint.c:394
#32 0x081ffeaa in Ffuncall (nargs=4, args=0xbffff00c) at eval.c:2733
#33 0x0823d229 in exec_byte_code (bytestr=137858540, vector=137858557, 
    maxdepth=54, args_template=4102, nargs=1, args=0xbffff340)
    at bytecode.c:919
#34 0x082005c9 in funcall_lambda (fun=137858517, nargs=1, 
    arg_vector=0xbffff33c) at eval.c:2893
#35 0x082000d6 in Ffuncall (nargs=2, args=0xbffff338) at eval.c:2775
#36 0x081ff9b3 in call1 (fn=7296, arg1=744440) at eval.c:2581
#37 0x08173963 in command_loop_1 () at keyboard.c:1520
#38 0x081fd28a in internal_condition_case (bfun=0x8173170 <command_loop_1>, 
    handlers=9336, hfun=0x8172986 <cmd_error>) at eval.c:1356
#39 0x08172e92 in command_loop_2 (ignore=0) at keyboard.c:1138
#40 0x081fcb94 in internal_catch (tag=22248, func=0x8172e67 <command_loop_2>, 
    arg=0) at eval.c:1116
#41 0x08172e2c in command_loop () at keyboard.c:1117
#42 0x081725e5 in recursive_edit_1 () at keyboard.c:723
#43 0x08172757 in Frecursive_edit () at keyboard.c:794
#44 0x08170748 in main (argc=2, argv=0xbffff664) at emacs.c:1629
Undefined command: "xback".  Try "help".
Warning: /home/rms/../lwlib: No such file or directory.
SIGINT is used by the debugger.
Are you sure you want to change it? (y or n) [answered Y; input not from terminal]
Environment variable "DISPLAY" not defined.
TERM = linux
Breakpoint 1 at 0x816ed85: file emacs.c, line 353.
Temporary breakpoint 2 at 0x818f7e0: file sysdep.c, line 903.
"write-region" (0xbfffe1d0)
"let*" (0xbfffe2c4)
"my-send-1" (0xbfffe3f0)
"if" (0xbfffe5a4)
"let" (0xbfffe724)
"save-excursion" (0xbfffe844)
"my-send-it" (0xbfffea08)
"mail-send" (0xbfffef2c)
"funcall-interactively" (0xbfffef28)
"call-interactively" (0xbffff010)
"command-execute" (0xbffff33c)

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: 21602@debbugs.gnu.org
> Date: Fri, 02 Oct 2015 21:37:48 -0400
> 
> I can reproduce the failure in emacs -Q with the code below.  The code
> in my-send-1 is responsible for passing specifying the coding system,
> but even if that value is wrong, write-region should not crash.

Thanks.  For the record, here's a simpler way of triggering the same
crash:

  emacs -Q

Type in *scratch*:

  (let ((coding-system-for-write (intern "\"us-ascii\"")))
    (write-region 1 10 "/tmp/SEGV"))

Evaluate this sexp, and you get the same segfault.

The problem here is that we don't check the validity of the
coding-system the user forced on us, until it's too late.

I found a couple more cases of missing validation like this one, and
fixed them all in commit 658f2c4.  The diffs are below.

diff --git a/src/fileio.c b/src/fileio.c
index e4b255a..65aaf57 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3470,7 +3470,10 @@ by calling `format-decode', which see.  */)
       mtime = time_error_value (save_errno);
       st.st_size = -1;
       if (!NILP (Vcoding_system_for_read))
-	Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
+	{
+	  CHECK_CODING_SYSTEM (Vcoding_system_for_read);
+	  Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
+	}
       goto notfound;
     }
 
@@ -4526,6 +4529,7 @@ choose_write_coding_system (Lisp_Object start, Lisp_Object end, Lisp_Object file
   else if (!NILP (Vcoding_system_for_write))
     {
       val = Vcoding_system_for_write;
+      CHECK_CODING_SYSTEM (val);
       if (coding_system_require_warning
 	  && !NILP (Ffboundp (Vselect_safe_coding_system_function)))
 	/* Confirm that VAL can surely encode the current region.  */
@@ -4574,6 +4578,9 @@ choose_write_coding_system (Lisp_Object start, Lisp_Object end, Lisp_Object file
 	  using_default_coding = 1;
 	}
 
+      if (!NILP (val))
+	CHECK_CODING_SYSTEM (val);
+
       if (! NILP (val) && ! force_raw_text)
 	{
 	  Lisp_Object spec, attrs;

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: 21602@debbugs.gnu.org
> Date: Fri, 02 Oct 2015 21:37:49 -0400
> 
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> 
> C backtrace followed by Lisp backtrace

Thanks, I see exactly the same with your reproduction recipe.

bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> @@ -4526,6 +4529,7 @@ choose_write_coding_system (Lisp_Object start, Lisp_Object end, Lisp_Object file
>    else if (!NILP (Vcoding_system_for_write))
>      {
>        val = Vcoding_system_for_write;
> +      CHECK_CODING_SYSTEM (val);
>        if (coding_system_require_warning
>  	  && !NILP (Ffboundp (Vselect_safe_coding_system_function)))
>  	/* Confirm that VAL can surely encode the current region.  */
> @@ -4574,6 +4578,9 @@ choose_write_coding_system (Lisp_Object start, Lisp_Object end, Lisp_Object file
>  	  using_default_coding = 1;
>  	}
>  
> +      if (!NILP (val))
> +	CHECK_CODING_SYSTEM (val);
> +
>        if (! NILP (val) && ! force_raw_text)
>  	{
>  	  Lisp_Object spec, attrs;

I think you also have to recheck val after calling
select-safe-coding-system, and default-buffer-file-coding-system.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: eliz@gnu.org,  rms@gnu.org
> Date: Sat, 03 Oct 2015 13:30:43 +0200
> 
> I think you also have to recheck val after calling
> select-safe-coding-system, and default-buffer-file-coding-system.

Right, fixed.

Thanks.

bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> diff --git a/src/fileio.c b/src/fileio.c
> index e4b255a..65aaf57 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -3470,7 +3470,10 @@ by calling `format-decode', which see.  */)
>        mtime = time_error_value (save_errno);
>        st.st_size = -1;
>        if (!NILP (Vcoding_system_for_read))
> -	Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
> +	{
> +	  CHECK_CODING_SYSTEM (Vcoding_system_for_read);
> +	  Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
> +	}

This is not needed, it is already checked later.  If
buffer-file-coding-system cannot tolerate a non-coding-system then
checks must be added somewhere else.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andreas Schwab <schwab@linux-m68k.org>
>> Cc: eliz@gnu.org,  rms@gnu.org
>> Date: Sat, 03 Oct 2015 13:30:43 +0200
>> 
>> I think you also have to recheck val after calling
>> select-safe-coding-system, and default-buffer-file-coding-system.
>
> Right, fixed.

That still misses a few checks, and left some redundant checks.  I think
it is better to put the checks in coding_inherit_eol_type, that will
cover almost all cases.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: eliz@gnu.org,  rms@gnu.org
> Date: Sat, 03 Oct 2015 14:07:19 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > diff --git a/src/fileio.c b/src/fileio.c
> > index e4b255a..65aaf57 100644
> > --- a/src/fileio.c
> > +++ b/src/fileio.c
> > @@ -3470,7 +3470,10 @@ by calling `format-decode', which see.  */)
> >        mtime = time_error_value (save_errno);
> >        st.st_size = -1;
> >        if (!NILP (Vcoding_system_for_read))
> > -	Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
> > +	{
> > +	  CHECK_CODING_SYSTEM (Vcoding_system_for_read);
> > +	  Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
> > +	}
> 
> This is not needed, it is already checked later.  If
> buffer-file-coding-system cannot tolerate a non-coding-system then
> checks must be added somewhere else.

Why would it be a bad idea to disallow setting of
buffer-file-coding-system to an invalid value, as a subtle side effect
of calling this function?  By the time that later check is made, the
damage is already done.

IOW, why is it bad to be more defensive than is strictly necessary?

bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> Why would it be a bad idea to disallow setting of
> buffer-file-coding-system to an invalid value, as a subtle side effect
> of calling this function?  By the time that later check is made, the
> damage is already done.
>
> IOW, why is it bad to be more defensive than is strictly necessary?

buffer-file-coding-system is a user-settable variable, so the check is
useless.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: 21602@debbugs.gnu.org,  rms@gnu.org
> Date: Sat, 03 Oct 2015 14:25:23 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Why would it be a bad idea to disallow setting of
> > buffer-file-coding-system to an invalid value, as a subtle side effect
> > of calling this function?  By the time that later check is made, the
> > damage is already done.
> >
> > IOW, why is it bad to be more defensive than is strictly necessary?
> 
> buffer-file-coding-system is a user-settable variable, so the check is
> useless.

In this case, we set it programmatically, so the user could rightfully
blame us.

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: 21602@debbugs.gnu.org,  rms@gnu.org
> Date: Sat, 03 Oct 2015 14:14:43 +0200
> 
> That still misses a few checks, and left some redundant checks.  I think
> it is better to put the checks in coding_inherit_eol_type, that will
> cover almost all cases.

I added the checks in coding_inherit_eol_type, but I think it's better
to keep the ones we already have.  In general, the sooner we detect
invalid arguments the better, especially when we are using a large
number of defaults of of many kinds, something the user might not be
aware of.

Thanks.

bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> In this case, we set it programmatically, so the user could rightfully
> blame us.

The only one to blame is the one who sets coding-system-for-read.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> I added the checks in coding_inherit_eol_type, but I think it's better
> to keep the ones we already have.  In general, the sooner we detect
> invalid arguments the better, especially when we are using a large
> number of defaults of of many kinds, something the user might not be
> aware of.

It doesn't make any sense to check both in the caller and the callee.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: 21602@debbugs.gnu.org,  rms@gnu.org
> Date: Sat, 03 Oct 2015 14:48:26 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > In this case, we set it programmatically, so the user could rightfully
> > blame us.
> 
> The only one to blame is the one who sets coding-system-for-read.

Indeed.  But that one could be some Lisp about which the user has no
intimate knowledge.

bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: 21602@debbugs.gnu.org,  rms@gnu.org
> Date: Sat, 03 Oct 2015 14:50:18 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I added the checks in coding_inherit_eol_type, but I think it's better
> > to keep the ones we already have.  In general, the sooner we detect
> > invalid arguments the better, especially when we are using a large
> > number of defaults of of many kinds, something the user might not be
> > aware of.
> 
> It doesn't make any sense to check both in the caller and the callee.

We do that all the time, because the same callee could be called by
some other caller, now or in the future.

Re: bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

Please revert b235cea, or I will.

This kind of conduct, where the change you think should be made was
explicitly objected to, but you still make it on your own, is not to
be tolerated here.  It's not your private project.  If you are
unconvinced, please feel free to try to convince the disagreeing
parties, instead of abusing your write access.

TIA

Re: bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

It is pretty stupid to check before the call if the callee does it
immediately again.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: emacs-devel@gnu.org
> Date: Sat, 03 Oct 2015 17:00:36 +0200
> 
> It is pretty stupid to check before the call if the callee does it
> immediately again.

I already explained why I don't consider this stupid.  I'm open to
hear additional arguments to countermand what I said ("stupid" doesn't
cut it), but only after you revert the parts of your commit that
changed what you consider "stupid" (I have nothing against the first 2
hunks).  I might even change my mind if you give convincing arguments.

There should be some minimal level of civilized behavior here.

Re: bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

It is wrong to error out if select-safe-coding-system-function returns
nil.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: bug#21602: 25.0.50; coding system seg fault

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: emacs-devel@gnu.org
> Date: Sat, 03 Oct 2015 18:09:05 +0200
> 
> It is wrong to error out if select-safe-coding-system-function returns
> nil.

Agreed (I didn't think CHECK_CODING_SYSTEM will barf in that case).
So the result should only checked if it's non-nil.

Re: bug#21602: 25.0.50; coding system seg fault

[Zack Piper]

On Sat, Oct 03, 2015 at 05:19:57PM +0300, Eli Zaretskii wrote:
> Please revert b235cea, or I will.
> 
> This kind of conduct, where the change you think should be made was
> explicitly objected to, but you still make it on your own, is not to
> be tolerated here.  It's not your private project.  If you are
> unconvinced, please feel free to try to convince the disagreeing
> parties, instead of abusing your write access.
> 
> TIA
> 

That commit caused:


Debugger entered--Lisp error: (wrong-type-argument coding-system-p
nil)
call-process("editorconfig" nil "ecbuffer" nil
"/home/zack/.emacs.d/config.org")
edconf-get-properties()
edconf-find-file-hook()
run-hooks(find-file-hook)
after-find-file(nil t)
find-file-noselect-1(#<buffer config.org>
"~/.emacs.d/config.org" nil nil
"~/.homesick/repos/dotfiles/home/.emacs.d/config.org"
(559853$
find-file-noselect("~/.emacs.d/config.org" nil nil nil)
find-file("~/.emacs.d/config.org")
org-babel-tangle-file("~/.emacs.d/config.org"
"~/.emacs.d/config.el" "emacs-lisp")
org-babel-load-file("~/.emacs.d/config.org")
eval-buffer(#<buffer  *load*> nil
"/home/zack/.emacs.d/init.el" nil t)  ; Reading
at buffer position 2342
load-with-code-conversion("/home/zack/.emacs.d/init.el"
"/home/zack/.emacs.d/init.el" t t)
load("/home/zack/.emacs.d/init" t t)
))

For me at least.

-- 
Zack Piper <zack@apertron.net> http://apertron.net

Re: bug#21602: 25.0.50; coding system seg fault

[Andreas Schwab]

Zack Piper <zack@apertron.net> writes:

> That commit caused:
>
>
> Debugger entered--Lisp error: (wrong-type-argument coding-system-p
> nil)

Sorry, I've removed that check again.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."