Re: PATCH 16/17: compctl: Comparing array with 0 does nothing, check contents (should it be dropped instead?)

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Tue, 6 Jan 2015 06:25:48 +0100
Mikael Magnusson <mikachu@xxxxxxxxx> 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