Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

[PATCH] Fix a bunch of Coverity-reported defects



I triaged about 85 defects in the Coverity scan UI.  The majority of
them were spurious, and I marked them "Ignore".  There were 14 that I
felt worthy of small fixes; those are included in the patch below.  I
believe that leaves 14 others where I wasn't confident of a fix;
several  of them are in zftp, as I recall.

A batch of the warnings that I ignored were assignments of one field
of a union to another field of the same union, e.g., a casted long
onto a double, etc., which elicited "overlapping copy" warnings.  I'm
fairly confident we'd have seen things crashing by now if this wasn't
safe, but I mention it in case someone knows why it might be a
problem.

One of those I did NOT fix is this, mentioned recently:

> > *** CID 1547827:  Null pointer dereferences  (FORWARD_NULL)
> > /Src/Modules/pcre.c: 370 in bin_pcre_match()
> > >>>     Passing null pointer "named" to "zpcre_get_substrings", which dereferences it.
>
> This is from Oliver's 51738 (PCRE's alternative DFA), I'm not going to
> interpret futher.

Let me know if there's anything controversial here.
index 8a7d0a4c5..8b863d5c8 100644
--- a/Src/Modules/zutil.c
+++ b/Src/Modules/zutil.c
@@ -1378,11 +1378,11 @@ rmatch(RParseResult *sm, char *subj, char *var1, char *var2, int comp)
 					     "zregexparse-guard"), !lastval))) {
 		LinkNode aln;
 		char **mend;
-		int len;
+		int len = 0;
 
 		queue_signals();
-		mend = getaparam("mend");
-		len = atoi(mend[0]);
+		if ((mend = getaparam("mend")))
+		    len = atoi(mend[0]);
 		unqueue_signals();
 
 		for (i = len; i; i--)
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 77fce66e8..9b87cad93 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -2249,8 +2249,9 @@ addmatches(Cadata dat, char **argv)
 	    llpl = strlen(lpre);
 	    llsl = strlen(lsuf);
 
-	    if (llpl + (int)strlen(compqiprefix) + (int)strlen(lipre) != origlpre
-	     || llsl + (int)strlen(compqisuffix) + (int)strlen(lisuf) != origlsuf)
+	    /* This used to reference compqiprefix and compqisuffix, why? */
+	    if (llpl + (int)strlen(qipre) + (int)strlen(lipre) != origlpre
+	     || llsl + (int)strlen(qisuf) + (int)strlen(lisuf) != origlsuf)
 		lenchanged = 1;
 
 	    /* Test if there is an existing -P prefix. */
diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
index 57789c0f3..cd8c7dd64 100644
--- a/Src/Zle/compresult.c
+++ b/Src/Zle/compresult.c
@@ -897,7 +897,7 @@ void
 do_allmatches(UNUSED(int end))
 {
     int first = 1, nm = nmatches - 1, omc = menucmp, oma = menuacc, e;
-    Cmatch *mc;
+    Cmatch *mc = 0;
     struct menuinfo mi;
     char *p = (brbeg ? ztrdup(lastbrbeg->str) : NULL);
 
@@ -915,10 +915,10 @@ do_allmatches(UNUSED(int end))
 #endif
     }
 
+    if (minfo.group)
+	mc = (minfo.group)->matches;
 
-    mc = (minfo.group)->matches;
-
-    while (1) {
+    while (mc) {
 	if (!((*mc)->flags & CMF_ALL)) {
 	    if (!first)
 		accept_last();
@@ -1731,8 +1731,6 @@ calclist(int showall)
                                  width < zterm_columns && nth < g->dcount;
                                  nth++, tcol++) {
 
-                                m = *p;
-
                                 if (tcol == tcols) {
                                     tcol = 0;
                                     tlines++;
@@ -1994,7 +1992,6 @@ printlist(int over, CLPrintFunc printm, int showall)
 		     (listdat.onlyexpl & ((*e)->always > 0 ? 2 : 1)))) {
 		    if (pnl) {
 			putc('\n', shout);
-			pnl = 0;
 			ml++;
 			if (cl >= 0 && --cl <= 1) {
 			    cl = -1;
@@ -2087,7 +2084,6 @@ printlist(int over, CLPrintFunc printm, int showall)
                         (showall || !(m->flags & (CMF_HIDE|CMF_NOLIST)))) {
 			if (pnl) {
 			    putc('\n', shout);
-			    pnl = 0;
 			    ml++;
 			    if (cl >= 0 && --cl <= 1) {
 				cl = -1;
diff --git a/Src/builtin.c b/Src/builtin.c
index 31af66c7c..9e08a1dbc 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -6508,6 +6508,7 @@ bin_read(char *name, char **args, Options ops, UNUSED(int func))
 
     if (OPT_ISSET(ops,'s') && SHTTY == readfd) {
 	struct ttyinfo ti;
+	memset(&ti, 0, sizeof(struct ttyinfo));
 	gettyinfo(&ti);
 	saveti = ti;
 	resettty = 1;
@@ -6606,7 +6607,8 @@ bin_read(char *name, char **args, Options ops, UNUSED(int func))
 		else if (resettty && SHTTY != -1)
 		    settyinfo(&saveti);
 		if (haso) {
-		    fclose(shout);
+		    if (shout)
+			fclose(shout);
 		    shout = oshout;
 		    SHTTY = -1;
 		}
diff --git a/Src/glob.c b/Src/glob.c
index 63f8a5fa7..bd199ace3 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -1317,7 +1317,8 @@ zglob(LinkList list, LinkNode np, int nountok)
 		sense = 0;
 		if (qualct) {
 		    qn = (struct qual *)hcalloc(sizeof *qn);
-		    qo->or = qn;
+		    if (qo)
+			qo->or = qn;
 		    qo = qn;
 		    qualorct++;
 		    qualct = 0;
diff --git a/Src/hist.c b/Src/hist.c
index bfbcd6ede..448dfddbc 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -1359,7 +1359,8 @@ putoldhistentryontop(short keep_going)
 	do {
 	    if (max_unique_ct-- <= 0 || he == hist_ring) {
 		max_unique_ct = 0;
-		he = hist_ring->down;
+		if (hist_ring)
+		    he = hist_ring->down;
 		next = hist_ring;
 		break;
 	    }
@@ -1367,12 +1368,16 @@ putoldhistentryontop(short keep_going)
 	    next = he->down;
 	} while (!(he->node.flags & HIST_DUP));
     }
-    if (he != hist_ring->down) {
+    /* Is it really possible for hist_ring to be NULL here? */
+    if (he && (!hist_ring || he != hist_ring->down)) {
 	he->up->down = he->down;
 	he->down->up = he->up;
 	he->up = hist_ring;
-	he->down = hist_ring->down;
-	hist_ring->down = he->down->up = he;
+	if (hist_ring) {
+	    he->down = hist_ring->down;
+	    hist_ring->down = he;
+	}
+	he->down->up = he;
     }
     hist_ring = he;
 }
@@ -1468,7 +1473,7 @@ should_ignore_line(Eprog prog)
 mod_export int
 hend(Eprog prog)
 {
-    int flag, hookret, stack_pos = histsave_stack_pos;
+    int flag, hookret = 0, stack_pos = histsave_stack_pos;
     /*
      * save:
      * 0: don't save
diff --git a/Src/input.c b/Src/input.c
index dd8f2edc7..d8ac2c0e7 100644
--- a/Src/input.c
+++ b/Src/input.c
@@ -643,18 +643,6 @@ zstuff(char **out, const char *fn)
     return len;
 }
 
-/**/
-char *
-ztuff(const char *fn)
-{
-    char *buf;
-    off_t len = zstuff(&buf, fn);
-    if (len > 0)
-	return buf;
-    else
-	return NULL;
-}
-
 /* stuff a whole file into the input queue and print it */
 
 /**/
diff --git a/Src/params.c b/Src/params.c
index 957656e3f..9f0cbcd67 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -6326,10 +6326,9 @@ mod_export Param
 upscope(Param pm, int reflevel)
 {
     Param up = pm->old;
-    while (pm && up && up->level >= reflevel) {
+    while (up && up->level >= reflevel) {
 	pm = up;
-	if (up)
-	    up = up->old;
+	up = up->old;
     }
     return pm;
 }
diff --git a/Src/utils.c b/Src/utils.c
index 790625379..0f66984cd 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -7523,8 +7523,8 @@ restoredir(struct dirsav *d)
     else if (d->level < 0)
 	err = -1;
     if (d->dev || d->ino) {
-	stat(".", &sbuf);
-	if (sbuf.st_ino != d->ino || sbuf.st_dev != d->dev)
+	if (stat(".", &sbuf) < 0 ||
+	    sbuf.st_ino != d->ino || sbuf.st_dev != d->dev)
 	    err = -2;
     }
     return err;


Messages sorted by: Reverse Date, Date, Thread, Author