Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
[PATCH] Fix a bunch of Coverity-reported defects
- X-seq: zsh-workers 52244
- From: Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>
- To: Zsh hackers list <zsh-workers@xxxxxxx>
- Subject: [PATCH] Fix a bunch of Coverity-reported defects
- Date: Wed, 25 Oct 2023 20:36:38 -0700
- Archived-at: <https://zsh.org/workers/52244>
- List-id: <zsh-workers.zsh.org>
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