From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs,gmane.emacs.devel Subject: bug#55719: [PATCH] bindat strz fixes Date: Wed, 01 Jun 2022 08:04:56 -0400 Message-ID: References: <6b1670d3-ae69-7f95-0e7d-d7cee0763c4a@rhansen.org> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28438"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 55719@debbugs.gnu.org, emacs-devel@gnu.org To: Richard Hansen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Jun 01 14:06:13 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 1nwN76-00073o-DI for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 01 Jun 2022 14:06:12 +0200 Original-Received: from localhost ([::1]:45538 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nwN75-0005dw-BW for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 01 Jun 2022 08:06:11 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:41138) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nwN6x-0005bH-8y for bug-gnu-emacs@gnu.org; Wed, 01 Jun 2022 08:06:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:55978) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nwN6w-0001Np-Tz for bug-gnu-emacs@gnu.org; Wed, 01 Jun 2022 08:06:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nwN6w-0006oc-J7 for bug-gnu-emacs@gnu.org; Wed, 01 Jun 2022 08:06:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 01 Jun 2022 12:06:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55719 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 55719-submit@debbugs.gnu.org id=B55719.165408511026118 (code B ref 55719); Wed, 01 Jun 2022 12:06:02 +0000 Original-Received: (at 55719) by debbugs.gnu.org; 1 Jun 2022 12:05:10 +0000 Original-Received: from localhost ([127.0.0.1]:49875 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nwN66-0006nC-8K for submit@debbugs.gnu.org; Wed, 01 Jun 2022 08:05:10 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:58100) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nwN62-0006mV-89 for 55719@debbugs.gnu.org; Wed, 01 Jun 2022 08:05:09 -0400 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 8A2A38075B; Wed, 1 Jun 2022 08:05:00 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id D69C7807CB; Wed, 1 Jun 2022 08:04:58 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1654085098; bh=HH/KVXXUqEDmW3DcrX9Xs4C2fwGrjz2fDyCVKzf2DAk=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=B0g7yR4YkaqfkiRxcu2T6x2K2c9Y/Mji+FW1xs0QREdCY/dJQ3ZFSla531tVV3S/+ Mi5T99ix+PLMOfj22WFkFyGHlYsF9flTfgO8pe7VHRrf5yv3K9cj8CfGI8BIE9rZNK fu8RCr01QtMzqdyikWjYTD8XE5ZJFaWqiFrBsOb1ZQv3cy8nFZnMkxzCcijG76hm0l hBKkztjhK/QJEqAkgobDdZpbbpuaHnODw7pcZuCZ+vXhLyY1mpxG2Hp/r74kE70M49 tXyuZF1Y/23DJRm/v1QnPFn5HV6MCeKygfUmI54Yozi7FFk12sVfVbsElYsY/V+aaH GBHVsrGDm/vtA== Original-Received: from pastel (unknown [45.72.221.51]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 36DAC120346; Wed, 1 Jun 2022 08:04:58 -0400 (EDT) In-Reply-To: (Richard Hansen's message of "Wed, 1 Jun 2022 01:28:06 -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:233484 gmane.emacs.devel:290486 Archived-At: >>> + (ert-deftest bindat-test--strz-fixedlen-pack-overflow () >>> + :expected-result :failed >>> + (should (equal (bindat-pack spec "abc") "\141\0"))) >> I think this changes the intended semantics. Until now `strz N` has >> meant that N bytes are used to encode the string and that it can >> hold upto a string of length N (in which case there's no terminating NUL >> byte). I agree that it's not the only valid semantics, but I'm not sure >> we want to change it at this point. >> Do you have a particular reason to make this change. > > A few: > * The documentation says that the packed output is null terminated > so that's what users expect. > * It is safer (packed output is less likely to cause some other > program to run past the end of the field). > * Without this change, there is no difference between `strz N` and > `str N`. So what would be the point of `strz N`? > * If the user selected strz, the application probably requires null > termination in all cases, not just when the string is under a > certain length. You're listing advantages of this choice, which we know about already. The other choice also has advantages. These don't count as "particular reasons" (e.g. a real-life concrete *need* for it, rather than a mere preference). The particular reason to prefer the current behavior is backward compatibility (which we could call "inertia"). Note also that `strz` without a length (or with a nil length) behaves the way you want. Of course, we could add an additional (optional) arg to `strz` to specify what should happen when unpacking a string with missing NUL byte as well as whether to truncate to N-1 chars rather than to N chars to make sure there is a terminating NUL byte. >>> + (ert-deftest bindat-test--strz-fixedlen-unpack () >>> + (should (equal (bindat-unpack spec "\0\0") "")) >>> + (should (equal (bindat-unpack spec "a\0") "a")))) >> How 'bout >> (bindat-unpack spec "ab") >> ? > > I added some comments explaining why cases like that aren't tested. The byte-string to unpack is not necessarily built from our own code (usually bindat is used to communicate with some other application), so whether our code can generate "ab" is not really relevant: the question still comes up about what we should do with "ab" (where valid answers could be to return "ab" or to return "a" or to signal an error, ...). Of course we can also decide it's "undefined". Stefan