From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <zsh-workers-return-34096-mason-zsh=primenet.com.au@zsh.org>
Received: (qmail 27855 invoked by alias); 5 Jan 2015 14:58:29 -0000
Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
X-No-Archive: yes
List-Id: Zsh Workers List <zsh-workers.zsh.org>
List-Post: <mailto:zsh-workers@zsh.org>
List-Help: <mailto:zsh-workers-help@zsh.org>
X-Seq: 34096
Received: (qmail 4759 invoked from network); 5 Jan 2015 14:58:28 -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-10-54aaa436a26e
Date: Mon, 05 Jan 2015 14:48:19 +0000
From: Peter Stephenson <p.stephenson@samsung.com>
To: zsh workers <zsh-workers@zsh.org>
Subject: Re: Fishy code in sticky emulation?
Message-id: <20150105144819.2f768fcb@pwslap01u.europe.root.pri>
In-reply-to:
 <CAHYJk3SjCocOpnoS5K_N=J7CWKmVBTgrUtryLLf+cu6xLqxZ8w@mail.gmail.com>
References: <CAHYJk3SjCocOpnoS5K_N=J7CWKmVBTgrUtryLLf+cu6xLqxZ8w@mail.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+NgFlrMLMWRmVeSWpSXmKPExsVy+t/xq7pmS1aFGDxdzGZxsPkhkwOjx6qD
	H5gCGKO4bFJSczLLUov07RK4MrasXs5a0MVf0bJxFVMD4yTuLkZODgkBE4m+jZ+YIGwxiQv3
	1rOB2EICSxklDk1x62LkArKXMEk0n21kg3C2MUrMffsVyOHgYBFQlXixwBSkgU3AUGLqptmM
	ILYIULj5+z8WEFtYQEdiat8ddpByXgF7iQ0NQiBhToFgibczFjBD7AqQuP/8BDuIzS+gL3H1
	L8w99hIzr5wBG8krICjxY/I9sJHMAloSm7c1sULY8hKb17yFmqMucePubvYJjEKzkLTMQtIy
	C0nLAkbmVYyiqaXJBcVJ6bmGesWJucWleel6yfm5mxghAftlB+PiY1aHGAU4GJV4eD1OrAwR
	Yk0sK67MPcQowcGsJML7Kn1ViBBvSmJlVWpRfnxRaU5q8SFGJg5OqQbG/NMXNx+/ucKJr7P1
	yM2jiY8Ft538yf1pWU2z96pczX9uizkt/kqsqvE6ZxPL7CmjE9DLqbXEQ2aStv2WAC6GR/U2
	KppCXwNcJyd5xjZcqTkctcVG1fOw6fIX1ls5d83p1K0/39e63fEwi1FswJUVy3Nb0jZff2wQ
	dW6pVpnizmvfGzQ6KgqUWIozEg21mIuKEwGhOMcRNgIAAA==

On Mon, 5 Jan 2015 15:34:00 +0100
Mikael Magnusson <mikachu@gmail.com> 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.

pws