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#55952: [PATCH] bindat (strz): Write null terminator after variable length string Date: Thu, 16 Jun 2022 10:15:40 +0300 Message-ID: <83fsk5qdsz.fsf@gnu.org> References: <95d2ffc9-ac61-91fa-605e-f95507f81217@rhansen.org> <834k0ntnkd.fsf@gnu.org> <90287bcd-b59b-e9e0-e828-0554a3f32e60@rhansen.org> <83fsk6rujd.fsf@gnu.org> <3fe77a58-74ab-d44f-1fd8-19cbf0b2bda3@rhansen.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="30028"; mail-complaints-to="usenet@ciao.gmane.io" Cc: monnier@iro.umontreal.ca, 55952-done@debbugs.gnu.org To: Richard Hansen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Jun 16 09:17:04 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 1o1jkV-0007aF-2i for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 16 Jun 2022 09:17:03 +0200 Original-Received: from localhost ([::1]:43072 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o1jkT-0006c7-8u for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 16 Jun 2022 03:17:01 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:51346) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o1jjX-0006bH-EM for bug-gnu-emacs@gnu.org; Thu, 16 Jun 2022 03:16:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:46662) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1o1jjW-0000DM-0p for bug-gnu-emacs@gnu.org; Thu, 16 Jun 2022 03:16:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1o1jjV-0000u1-Qe for bug-gnu-emacs@gnu.org; Thu, 16 Jun 2022 03:16:01 -0400 Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-To: bug-gnu-emacs@gnu.org Resent-Date: Thu, 16 Jun 2022 07:16:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: cc-closed 55952 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Mail-Followup-To: 55952@debbugs.gnu.org, eliz@gnu.org, rhansen@rhansen.org Original-Received: via spool by 55952-done@debbugs.gnu.org id=D55952.16553637573454 (code D ref 55952); Thu, 16 Jun 2022 07:16:01 +0000 Original-Received: (at 55952-done) by debbugs.gnu.org; 16 Jun 2022 07:15:57 +0000 Original-Received: from localhost ([127.0.0.1]:40558 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o1jjR-0000te-7M for submit@debbugs.gnu.org; Thu, 16 Jun 2022 03:15:57 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:38120) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o1jjO-0000tO-6q for 55952-done@debbugs.gnu.org; Thu, 16 Jun 2022 03:15:55 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:54592) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o1jjH-0000Bj-GZ; Thu, 16 Jun 2022 03:15:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=iUXNgowuqdO3S8ith3xYZklRUyjEJ7jArEqeqKW9BYM=; b=C5ueWWrqbYp6 mn6Zz+/qSG5hZ/LouUw2OqWJxrib7TdETML5XG5W+aPi2i4U4HP+ESnG/3DYTZSAbeBcCUE5sSTWF qteTKyebB9xenKmi2rtooHNWaNTfoIgs0J3He3jSXy1SBJgfjnlhMjIqA02mIZiqrKrhAoYahafD6 OyjGk+JDygQS8GHL9HvEVfvoGOG/ViL/fPf5DptTTQN+jKcxXucHxyHNbiJpZQKnN5cy7sX7rF/hZ /RyZuDIpx4+wm5LzqqYomCXHSg6q3LFOtCi/BtFx9OmnZ118hpVfD0dSHbBHlK7HOwvWXvde6T+vo oP0coH598Z16rCYmnHpLCg==; Original-Received: from [87.69.77.57] (port=1450 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 1o1jjG-0004sT-TS; Thu, 16 Jun 2022 03:15:47 -0400 In-Reply-To: <3fe77a58-74ab-d44f-1fd8-19cbf0b2bda3@rhansen.org> (message from Richard Hansen on Wed, 15 Jun 2022 14:49:58 -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:234622 Archived-At: > Date: Wed, 15 Jun 2022 14:49:58 -0400 > Cc: 55952@debbugs.gnu.org, monnier@iro.umontreal.ca > From: Richard Hansen > > >> The existing code advances the index for the terminator, it just > >> doesn't write 0 to that byte. So the existing code already signals > >> an error in that case unless the `strz` is the final field. > > > > I don't see how this is relevant to the concern I expressed. > > The point I was trying to make is that this patch doesn't change the behavior (in any significant way) in the case of an undersized output string. Perhaps the documentation could be improved, but I'd rather do that in a follow-up patch because it is outside of the scope of this patch. I never had any objections to changing the behavior, nor in general to the code changes in your patch. My comments were only about the documentation in the manual. > > My concern is that you found it necessary to add a comment about > > writing the terminating null byte (which is a good thing), but didn't > > mention that aspect in the manual, not even as a hint. I think it is > > noteworthy enough to be in the manual. > > What do you mean? I meant that the caveat about having enough space in the output string for the terminating null byte is not explicitly mentioned where strz and its handling during packing are documented. > > Bottom line: I think this aspect of the code is important to mention > > in the manual. The price is small, whereas the benefit could be > > significant. > > I disagree -- I think the price is relatively high compared to the benefit. The pre-allocated length requirement is a requirement of only `bindat-pack` -- not of `bindat-type` or any of the type specifiers -- so it is best to keep that requirement with the documentation of `bindat-pack`. Indeed, users are unaware that packing to a pre-allocated string is even possible until they read the documentation for `bindat-pack` (except for references in the caveats documented for fixed-length `str` and `strz`, which I plan on addressing in a future patch). I don't see any point in continuing to argue about this. I have my opinions about how our manuals should explain these issues, and your disagreement, which is noted, doesn't change those opinions. So I installed your previous version, and then made the changes in the manual I thought were pertinent. Thanks.