From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12943 invoked by alias); 5 Jan 2015 23:21: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: 34097 Received: (qmail 8419 invoked from network); 5 Jan 2015 23:21:08 -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=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=f19Rlr7cgotde0fG/IqOy8f2VgyNF+uoaD8efBnaJls=; b=tI/7L90X0c4lo63/Z5sL9VyNd275nv+bij5X4jt8aTsRP27oJNmxh+GejOUj1GotSv Vi6CUhx4/H8QOvUqTRPZneNISKtdm5TDeBxVv3JXB1HvtptX8G6uBeLboWCvAno43uGq 919gq0PQFsunC/xMzlYMY6vVXWXwHNzcJu+MnKtLl3Lgp+vooK/81ajftW/jRH7+7neh IEsbZWdOIG0ze1/woM2ssHQMOFmnpQX24tUuqIqTWtuJNgAjgNWGw+x4mR88NqUkJWDu sFlFVJP1X3+byEFFk8LdrTrj2FG/w0KbMDIpJRl46uk1gnQzLn0yopxtXlcuBx9q6wVT a78A== MIME-Version: 1.0 X-Received: by 10.50.138.226 with SMTP id qt2mr16775249igb.1.1420500064417; Mon, 05 Jan 2015 15:21:04 -0800 (PST) In-Reply-To: <20150105144819.2f768fcb@pwslap01u.europe.root.pri> References: <20150105144819.2f768fcb@pwslap01u.europe.root.pri> Date: Tue, 6 Jan 2015 00:21:04 +0100 Message-ID: Subject: Re: Fishy code in sticky emulation? From: Mikael Magnusson To: Peter Stephenson Cc: zsh workers Content-Type: text/plain; charset=UTF-8 On Mon, Jan 5, 2015 at 3:48 PM, Peter Stephenson wrote: > On Mon, 5 Jan 2015 15:34:00 +0100 > Mikael Magnusson wrote: >> I'm looking through Coverity issues (some patches to come later), and >> it flagged this in builtin.c that I can't quite say for sure if it's >> right or wrong about. >> >> int >> bin_emulate(UNUSED(char *nam), char **argv, Options ops, UNUSED(int func)) >> { >> ... >> if (sticky->n_on_opts) >> on_ptr = sticky->on_opts = >> zhalloc(sticky->n_on_opts * sizeof(*sticky->on_opts)); >> else >> on_ptr = NULL; >> if (sticky->n_off_opts) >> off_ptr = sticky->off_opts = zhalloc(sticky->n_off_opts * >> sizeof(*sticky->off_opts)); >> else >> off_ptr = NULL; >> for (optnode = firstnode(optlist); optnode; incnode(optnode)) { >> /* Data is index into new_opts */ >> char *optptr = (char *)getdata(optnode); >> int optno = optptr - new_opts; >> if (*optptr) >> *on_ptr++ = optno; >> else >> *off_ptr++ = optno; >> } >> ... >> >> In particular, on_ptr and off_ptr can be NULL, but unconditionally one >> of them is always incremented in the for loop, which isn't very well >> defined for a NULL pointer. Am I missing something, or are these >> n_*_opts simply never 0? > > You missed out the previous loop which is exactly parallel to the second > one you quoted: > > for (optnode = firstnode(optlist); optnode; incnode(optnode)) { > /* Data is index into new_opts */ > char *optptr = (char *)getdata(optnode); > if (*optptr) > sticky->n_on_opts++; > else > sticky->n_off_opts++; > } > > This means that in the second loop on_ptr must be non-NULL whenever > *optptr is non-NULL and off_ptr must be non-NULL whenever it's NULL. > > However, it would be fine to make the test for the pointers more > explicit; it's not going to make any noticeable difference even if you > run emulate very frequently. Ah, oops. I guess the getdata() functions are too involved for coverity to follow (it gets confused by way easier things than that too), and for me too :). Thanks, I'll mark it as a false positive. -- Mikael Magnusson