From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id 42150496 for ; Sat, 29 Dec 2018 09:55:47 +0000 (UTC) Received: (qmail 615 invoked by alias); 29 Dec 2018 09:55:31 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 43951 Received: (qmail 13814 invoked by uid 1010); 29 Dec 2018 09:55:30 -0000 X-Qmail-Scanner-Diagnostics: from out1-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.100.2/25112. spamassassin: 3.4.2. Clear:RC:0(66.111.4.25):SA:0(-1.1/5.0):. Processed in 5.865652 secs); 29 Dec 2018 09:55:30 -0000 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= daniel.shahaf.name; h=date:from:to:subject:message-id:references :mime-version:content-type:content-transfer-encoding :in-reply-to; s=fm2; bh=TiInBN+o2FyeNwy+zkC+PKq8ADoS6W6/zwSp7GVv VUg=; b=DVTqE1Pf6wb5Bfws5nNaRIBvJBUZqrxAjuWfLwdfE67apNMirNU490qf HrAB8v5Oreo+Ejc7Q6i5eAEsPN616aUHF7VpAsxTqkSw95DEeJVOkxhSP3Nv8660 rhmMiyI6xee5Fdvjdp5llFfU4O96xxvEnGb0eqgmP5GepTWMCW9Qem/c6YF3VBBp tfIsmnUfnSbAHPwfNtncHdTJKNJp9Eiwzw+7OvXqpk1p/nUGedU02Nq/JVMoLps/ e0vPSHFOZjkJUKbGr9H+J3hqb4NscKKcAoaNJ5pbySe2kUSv4QQUZtgnHOHefIWZ qamjQBtUK9AoZiHaYsPa+/juza1EzQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=TiInBN+o2FyeNwy+zkC+PKq8ADoS6W6/zwSp7GVvV Ug=; b=CW96SnvGfTUP/yJjs5uQ4/yK2+FOk9+uAyje5SWmRVO75occHJViOiM+o BquGN38mojSPc+IQMReTVPk7FqpBSWIHWPeG7++O2Zj1oF8RJn/gehGbx6u1PaAK bPPrrkauq2WT7hPHo9CGR1mP0iq4WM7iWmBEKYXqfAbfsVqMmlZs5VCWoFDx/gMt fjoyNwrj/DSOYrhBAZ0pTG8xpzxONFk9X8/aaTtDs/Ci6lj96EH6NNw4NAAl52xb VnTaMXECXsfvk6tn/+2IRAaUd28EI4HW0amk4alG9MU9JZD9vXUrUON8pCBrAn4G kwN8oZUCqaodmRh3YBiYoAgIKAuyg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedtledrtdekgddutdculddtuddrgedtkedrtddtmd cutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfhuthen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtugfgjggfsehtkedttddtreejnecuhfhrohhmpeffrghn ihgvlhcuufhhrghhrghfuceougdrshesuggrnhhivghlrdhshhgrhhgrfhdrnhgrmhgvqe enucfkphepjeelrddukeefrdegkedrudelgeenucfrrghrrghmpehmrghilhhfrhhomhep ugdrshesuggrnhhivghlrdhshhgrhhgrfhdrnhgrmhgvnecuvehluhhsthgvrhfuihiivg eptd X-ME-Proxy: Date: Sat, 29 Dec 2018 09:55:16 +0000 From: Daniel Shahaf To: Zsh workers Subject: Re: [PATCH] ztrftime(): Fix truncation for %. Message-ID: <20181229095516.npdfdskue6yhnrtr@tarpaulin.shahaf.local2> References: <20181224054021.GK1941@sym.noone.org> <20181224071421.GL1941@sym.noone.org> <1545655545.1499531.1617344072.151563DD@webmail.messagingengine.com> <4E316F0B-5606-4E93-8988-28A5444612E6@dana.is> <20181224170601.i7fiz4zv5fboqtrw@tarpaulin.shahaf.local2> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) dana wrote on Fri, Dec 28, 2018 at 16:16:03 -0600: > On 24 Dec 2018, at 11:31, dana wrote: > >Think i'll revisit it after Christmas :/ > > Maybe this is more reasonable...? It's 0.100 correct. (Sorry, I meant to say "99%", but I printed that line with unpatched master ☺) > +++ b/Src/utils.c > @@ -3334,19 +3334,27 @@ morefmt: > #endif > switch (*fmt++) { > case '.': > - if (ztrftimebuf(&bufsize, digs)) > - return -1; > + { > if (digs > 9) > digs = 9; > + if (ztrftimebuf(&bufsize, digs)) > + return -1; > + long fnsec = nsec; > if (digs < 9) { > - int trunc; > + int trunc, max = 10; > + for (trunc = 1; trunc < digs; trunc++) > + max *= 10; > + max -= 1; As a matter of idiom, it would be clearer to initialize max to 1 (the multiplicative identity) and trunc to 0 (so the loop body is run 'digs' times). Next, when «digs == 8», max will be multiplied by ten eight times (or initialized to 10 and multiplied by ten seven times); however, 10⁸ > INT32_MAX, so when 'int' is a 32-bit type (as on x86 in Axel's original report) max will overflow, wrap around, and not have the intended value. Using a 64-bit type (probably zlong) would address this — in that case don't forget to adjust the sprintf() format string. > for (trunc = 8 - digs; trunc; trunc--) > - nsec /= 10; > - nsec = (nsec + 8) / 10; > + fnsec /= 10; > + fnsec = (fnsec + 5) / 10; > + if (fnsec > max) > + fnsec = max; With the int size change, this should be correct; however, since 'fnsec' and 'max' are initialized separately, they could easily (after a future code change) end up being out of sync by an order of magnitude. That is: it's correct, but perhaps not as robust as it could be. As an alternative, you could initialize 'max' to «1000000000ull» and divide it by 10 in each iteration of the 'nsec' loop. (There are other ways but this one seems straightforward and doesn't run into wraparound edge cases.) > } > - sprintf(buf, "%0*ld", digs, nsec); > + sprintf(buf, "%0*ld", digs, fnsec); > buf += digs; > break; > + } > +++ b/Test/V09datetime.ztst > @@ -114,3 +114,17 @@ > + strftime '%Y-%m-%d %H:%M:%S.%3.' 1012615322 $(( 999_999 )) > + for 1 in %. %1. %3. %6. %9. %12.; do > + print -rn - "$1 " > + strftime "%Y-%m-%d %H:%M:%S.$1" 1012615322 $(( 999_999_999 )) > + done > +0:%. truncation You mentioned upthread that this fixes a bug related to %. occurring twice in the format string; add a test for that? Bottom line, just change the int type and this'll be good to go. ☺ Cheers, Daniel