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: Wed, 15 Jun 2022 15:16:38 +0300 Message-ID: <83fsk6rujd.fsf@gnu.org> References: <95d2ffc9-ac61-91fa-605e-f95507f81217@rhansen.org> <834k0ntnkd.fsf@gnu.org> <90287bcd-b59b-e9e0-e828-0554a3f32e60@rhansen.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40343"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 55952@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 Wed Jun 15 14:17:17 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 1o1RxU-000AJ1-PJ for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 15 Jun 2022 14:17:16 +0200 Original-Received: from localhost ([::1]:49346 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o1RxT-0005ub-7v for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 15 Jun 2022 08:17:15 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:51042) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o1RxG-0005uP-J8 for bug-gnu-emacs@gnu.org; Wed, 15 Jun 2022 08:17:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:42843) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1o1RxG-00012x-A8 for bug-gnu-emacs@gnu.org; Wed, 15 Jun 2022 08:17:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1o1RxG-0007Zc-5F for bug-gnu-emacs@gnu.org; Wed, 15 Jun 2022 08:17: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: Wed, 15 Jun 2022 12:17:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55952 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 55952-submit@debbugs.gnu.org id=B55952.165529541729095 (code B ref 55952); Wed, 15 Jun 2022 12:17:02 +0000 Original-Received: (at 55952) by debbugs.gnu.org; 15 Jun 2022 12:16:57 +0000 Original-Received: from localhost ([127.0.0.1]:36740 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o1RxA-0007ZD-Pl for submit@debbugs.gnu.org; Wed, 15 Jun 2022 08:16:57 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:37820) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o1Rx7-0007Yz-VP for 55952@debbugs.gnu.org; Wed, 15 Jun 2022 08:16:55 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:60088) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o1Rx1-00010u-Vb; Wed, 15 Jun 2022 08:16: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=WTzs0bS2sJWhZjxVlzVAKSHBNemVhNv9ioTGrT9tWL4=; b=CftiRqkXQTxU hJK7MD46pUErl3ypNsa69mC580QTxyLoFiNPkziY5SAahQSD7JnB8EIwiA09z5jrWOoUB4skdIwEE aaM2lmQe6YnTQxaMueFeb6CCh5QeLlaX5LBEJEJ24O/fAHBMfpsClph0rfXJmnPyUCWeGnTjdBflj lVr20U9ivmInScDL7i0JFJaLNqpdYBr6yNS7U0AYonHY5puJrurGszgDDD4FaYqDGkxwXJvHQhwla 878/zKJc9UDPVpA82XOv5wiNlz/Fackgr9oSSZjV+kAMU7nOkxs8uX4ELyzYFYHAalK0i5Db0qFTR wFfukXt9YED8c6t/l7MX/w==; Original-Received: from [87.69.77.57] (port=2964 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 1o1Rx1-00010C-BZ; Wed, 15 Jun 2022 08:16:47 -0400 In-Reply-To: <90287bcd-b59b-e9e0-e828-0554a3f32e60@rhansen.org> (message from Richard Hansen on Tue, 14 Jun 2022 16:47:47 -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:234569 Archived-At: > Date: Tue, 14 Jun 2022 16:47:47 -0400 > Cc: 55952@debbugs.gnu.org, monnier@iro.umontreal.ca > From: Richard Hansen > > > Thanks, but AFAICT the documentation doesn't describe accurately > > enough what the modified code does: what if the pre-allocated > > destination string doesn't have enough storage for the null byte the > > code adds? > > 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. 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. > Regardless, the documentation for `bindat-pack` [1] clearly states that the pre-allocated string must have enough room: > > > When pre-allocating, you should make sure `(length raw)` meets or > > exceeds the total length to avoid an out-of-range error. > [1] https://www.gnu.org/software/emacs/manual/html_node/elisp/Bindat-Functions.html#index-bindat_002dpack This comes _after_ the place where strz is described, so if someone reads the manual in order, they wouldn't have read that yet. And even if they did, there's no reason to assume they remember it well enough. 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. Thanks.