From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13152 invoked by alias); 6 Jan 2015 10:20:39 -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: 34130 Received: (qmail 24514 invoked from network); 6 Jan 2015 10:20:37 -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=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, SPF_HELO_PASS autolearn=ham version=3.3.2 X-AuditID: cbfec7f4-b7f126d000001e9a-f0-54abb6f2c083 Date: Tue, 06 Jan 2015 10:20:32 +0000 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: PATCH 16/17: compctl: Comparing array with 0 does nothing, check contents (should it be dropped instead?) Message-id: <20150106102032.35ba4a55@pwslap01u.europe.root.pri> In-reply-to: <1420521949-30483-17-git-send-email-mikachu@gmail.com> References: <1420521949-30483-1-git-send-email-mikachu@gmail.com> <1420521949-30483-17-git-send-email-mikachu@gmail.com> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrKLMWRmVeSWpSXmKPExsVy+t/xa7qftq0OMTiyS8fiYPNDJgdGj1UH PzAFMEZx2aSk5mSWpRbp2yVwZUzY3cBc8IevYtLPmywNjM+4uxg5OSQETCQWPH3FDmGLSVy4 t56ti5GLQ0hgKaNEw7Y2dghnCZNE35kWqMw2Rok9p7azgLSwCKhKbLm8nwnEZhMwlJi6aTYj iC0iIC5xdu15sBphgQqJtes6wFbwCthLdBxsYQWxOQWcJX6englUwwE0tEqi610ASJhfQF/i 6t9PTBAX2UvMvHKGEaJVUOLH5HtgI5kFtCQ2b2tihbDlJTavecsMYgsJqEvcuLubfQKj0Cwk LbOQtMxC0rKAkXkVo2hqaXJBcVJ6rqFecWJucWleul5yfu4mRkjQftnBuPiY1SFGAQ5GJR7e D+2rQ4RYE8uKK3MPMUpwMCuJ8O6YDhTiTUmsrEotyo8vKs1JLT7EyMTBKdXAuKJvDW+K3Z6Z f3csuBZosuiJmLLx9vs6i+w9N0insosv+vnS/Hl85MKMJbPrrx5yfLH62QaTvHU/Tj+cosG4 v4x19p6lBuE5V/Y3VWceWmm1UYD5mOr8TXqLVHateVXAW1rVE1GRUrRjn+pfw6LDjp8Y05ha tuf8ep+e9H7a1X+Jv/ateJ17yECJpTgj0VCLuag4EQBxlvK2OAIAAA== On Tue, 6 Jan 2015 06:25:48 +0100 Mikael Magnusson wrote: > Found by Coverity (Issue 1255780). > --- > Src/Zle/compctl.c | 2 +- > Src/jobs.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c > index 96ad6a2..2a80e6c 100644 > --- a/Src/Zle/compctl.c > +++ b/Src/Zle/compctl.c > @@ -3685,7 +3685,7 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd) > > for (i = 0; i <= maxjob; i++) > if ((jobtab[i].stat & STAT_INUSE) && > - jobtab[i].procs && jobtab[i].procs->text) { > + jobtab[i].procs && jobtab[i].procs->text[0]) { > int stopped = jobtab[i].stat & STAT_STOPPED; > > j = dupstring(jobtab[i].procs->text); > diff --git a/Src/jobs.c b/Src/jobs.c > index c6e1bce..295f4c9 100644 > --- a/Src/jobs.c > +++ b/Src/jobs.c > @@ -2718,7 +2718,7 @@ findjobnam(const char *s) > for (jobnum = maxjob; jobnum >= 0; jobnum--) > if (!(jobtab[jobnum].stat & (STAT_SUBJOB | STAT_NOPRINT)) && > jobtab[jobnum].stat && jobtab[jobnum].procs && jobnum != thisjob && > - jobtab[jobnum].procs->text && strpfx(s, jobtab[jobnum].procs->text)) > + jobtab[jobnum].procs->text[0] && strpfx(s, jobtab[jobnum].procs->text)) > return jobnum; > return -1; > } This is because text is an array within the structure. I haven't looked to see if we always sanitise the maximum length (including NULL) to JOBTEXTSIZE; I don't know if Coverity would know about that. We initialise it as if (text) strcpy(pn->text, text); else *pn->text = '\0'; So the change is fine but in the second case the remaining test might not be needed: the strpfx() will fail unless s is also empty and if it is also empty logically it should probably succeed (how useful that is is another question). But probably not worth worrying about. pws