From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#55897: [PATCH] bindat (str, strz): Convert to unibyte when packing Date: Sat, 11 Jun 2022 11:11:15 +0300 Message-ID: <83zgijy5zw.fsf@gnu.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="6491"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 55897@debbugs.gnu.org, monnier@iro.umontreal.ca To: Richard Hansen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jun 11 10:16:55 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nzwIh-0001V1-0N for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 11 Jun 2022 10:16:55 +0200 Original-Received: from localhost ([::1]:46592 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nzwIf-0005y6-I0 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 11 Jun 2022 04:16:53 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:56290) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nzwDy-0002KC-TM for bug-gnu-emacs@gnu.org; Sat, 11 Jun 2022 04:12:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57906) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nzwDy-0003g9-Jw for bug-gnu-emacs@gnu.org; Sat, 11 Jun 2022 04:12:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nzwDy-0003gT-CE for bug-gnu-emacs@gnu.org; Sat, 11 Jun 2022 04:12:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 11 Jun 2022 08:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55897 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 55897-submit@debbugs.gnu.org id=B55897.165493509214118 (code B ref 55897); Sat, 11 Jun 2022 08:12:02 +0000 Original-Received: (at 55897) by debbugs.gnu.org; 11 Jun 2022 08:11:32 +0000 Original-Received: from localhost ([127.0.0.1]:51803 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nzwDT-0003fd-Vg for submit@debbugs.gnu.org; Sat, 11 Jun 2022 04:11:32 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:43042) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nzwDP-0003fN-Kf for 55897@debbugs.gnu.org; Sat, 11 Jun 2022 04:11:31 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:51296) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nzwDK-0003Xv-7s; Sat, 11 Jun 2022 04:11:22 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=0y/S38upLlXMPkUiWM/331Cfu2qp0n47eUFwvrtSvcM=; b=a3Kg4ulx+K7TkA71+Uex iZXdum/PDXHUNc6joS883VoLhAyQonqxfrLxNC70W8QY2xU9eVSAca6m7G3PHaVV478aREH7K5pV4 8wEbsGCABO+CGgCIuXI6loOwikOFMyxKYbZiwMQHd1v18U7XPqvu1SlIlkyxHTKe2rdWHBumSA982 Tl+YMD3+MJwJQdQ3TUTtrkOgizvThAQvrKvQ4Qzw0n3+cPJAu8HYI5ZKbVGzjimkOscix455MlFa9 zE8V2XMkaBhqYzTKs1KNTbJecCSSoeXAduyqHKCxCCagNGlKD/wWhAZ0prR1n2o+Jm20Kpn0i59hI j4uGcp7hMu5Zbg==; Original-Received: from [87.69.77.57] (port=4929 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nzwDJ-0005Un-Nn; Sat, 11 Jun 2022 04:11:22 -0400 In-Reply-To: (message from Richard Hansen on Sat, 11 Jun 2022 00:38:00 -0400) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:234253 Archived-At: > Cc: monnier@iro.umontreal.ca > Date: Sat, 11 Jun 2022 00:38:00 -0400 > From: Richard Hansen > > (defun bindat--pack-str (len v) > + (if (multibyte-string-p v) > + (signal 'wrong-type-argument `(multibyte-string-p ,v))) Isn't this too strict? First, a string can be multibyte and pure-ASCII: (let ((str (decode-coding-string "abcde" 'utf-8))) (multibyte-string-p str)) => t Shouldn't it be possible to use such strings here? Furthermore, I think you said you wanted to extend bindat so it could use multibyte string that contain ASCII and eight-bit characters? If so, this sounds like shooting ourselves in the foot? > (defun bindat--pack-str (len v) > - (if (multibyte-string-p v) > - (signal 'wrong-type-argument `(multibyte-string-p ,v))) > - (dotimes (i (min len (length v))) > - (aset bindat-raw (+ bindat-idx i) (aref v i))) > - (setq bindat-idx (+ bindat-idx len))) > + (let ((v (string-to-unibyte v))) > + (dotimes (i (min len (length v))) > + (aset bindat-raw (+ bindat-idx i) (aref v i))) > + (setq bindat-idx (+ bindat-idx len)))) And here you remove that error back? Why does it make sense to introduce an error message, only to remove it in the very next commit? Please instead make a single change which incorporates both. > --- a/test/lisp/emacs-lisp/bindat-tests.el > +++ b/test/lisp/emacs-lisp/bindat-tests.el > @@ -193,13 +193,15 @@ bindat-test--str-strz-multibyte > (dolist (spec (list (bindat-type str 2) > (bindat-type strz 2) > (bindat-type strz))) > - (should-error (bindat-pack spec (string-to-multibyte "x"))) > - (should-error (bindat-pack spec (string-to-multibyte "\xff"))) > + (should (equal (bindat-pack spec (string-to-multibyte "x")) "x\0")) > + (should (equal (bindat-pack spec (string-to-multibyte "\xff")) "\xff\0")) > (should-error (bindat-pack spec "💩")) > (should-error (bindat-pack spec "\N{U+ff}"))) > (dolist (spec (list '((x str 2)) '((x strz 2)))) > - (should-error (bindat-pack spec `((x . ,(string-to-multibyte "x"))))) > - (should-error (bindat-pack spec `((x . ,(string-to-multibyte "\xff"))))) > + (should (equal (bindat-pack spec `((x . ,(string-to-multibyte "x")))) > + "x\0")) > + (should (equal (bindat-pack spec `((x . ,(string-to-multibyte "\xff")))) > + "\xff\0")) > (should-error (bindat-pack spec '((x . "💩")))) > (should-error (bindat-pack spec '((x . "\N{U+ff}")))))) Likewise here. Thanks. P.S. Please also mention the bug number in the log message of the next version of the patch, since the number is now known.