From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22592 invoked by alias); 27 Nov 2013 18:54:10 -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: X-Seq: 32061 Received: (qmail 9215 invoked from network); 27 Nov 2013 18:54:04 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 From: Bart Schaefer Message-id: <131127105409.ZM10472@torch.brasslantern.com> Date: Wed, 27 Nov 2013 10:54:09 -0800 In-reply-to: <20131127180719.1ad6acf0@pwslap01u.europe.root.pri> Comments: In reply to Peter Stephenson "Re: PATCH: utils.c: Fix use of uninitialized memory in metafy()." (Nov 27, 6:07pm) References: <20131127180719.1ad6acf0@pwslap01u.europe.root.pri> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: Re: PATCH: utils.c: Fix use of uninitialized memory in metafy(). MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Nov 27, 6:07pm, Peter Stephenson wrote: } } Hmm... I think the intention probably *is* to check if there's null } termination at "buf + len", on the assumption that the first "len" bytes } need metafying regardless. So if we've got only len valid bytes, not } null-terminated (or null-terminated by accident because the next byte } that isn't actually valid for the allocation happens to be null), we've } got no way of knowing this given the current interface. Does it actually matter? The only reason for (*e != 0) as far as I can tell is to be sure we've actually done (*e = '\0') at the very end of the whole thing [comment: "... unchanged (a terminating null character is appended to buf if necessary)"]. Can't we just move the *e = '\0' outside the "if" body and skip the test in the condition? All tests still pass with the following: diff --git a/Src/utils.c b/Src/utils.c index 0db9c30..c6d178c 100644 --- a/Src/utils.c +++ b/Src/utils.c @@ -3985,7 +3985,7 @@ metafy(char *buf, int len, int heap) if (imeta(*e++)) meta++; - if (meta || heap == META_DUP || heap == META_HEAPDUP || *e != '\0') { + if (meta || heap == META_DUP || heap == META_HEAPDUP) { switch (heap) { case META_REALLOC: buf = zrealloc(buf, len + meta + 1); @@ -4028,8 +4028,8 @@ metafy(char *buf, int len, int heap) meta--; } } - *e = '\0'; } + *e = '\0'; return buf; }