static analysis for o*.c

This construct triggered several complaints about passing Null to
  Strcpy(simpleoname, obufp = the(simpleoname));
Changing that to
  obufp = the(simpleoname);
  Strcpy(simpleoname, obufp);
prevents it, but the original complaint is bogus and the "fix"
doesn't do anything to deal with Null arguments.

A couple of other changes introduce different code in order to get
different behavior.  I updated from llvm-16 to llvm-19 but didn't
eliminate any of the spurious complaints.
This commit is contained in:
PatR
2025-01-21 22:42:23 -08:00
parent dc5c098cb5
commit 3109e706e9
2 changed files with 21 additions and 15 deletions
+17 -10
View File
@@ -32,9 +32,9 @@ struct _readobjnam_data {
char fruitbuf[BUFSZ]; char fruitbuf[BUFSZ];
}; };
staticfn char *strprepend(char *, const char *); staticfn char *strprepend(char *, const char *) NONNULL NONNULLARG1;
staticfn char *nextobuf(void); staticfn char *nextobuf(void) NONNULL;
staticfn void releaseobuf(char *); staticfn void releaseobuf(char *) NONNULLARG1;
staticfn void xcalled(char *, int, const char *, const char *); staticfn void xcalled(char *, int, const char *, const char *);
staticfn char *xname_flags(struct obj *, unsigned); staticfn char *xname_flags(struct obj *, unsigned);
staticfn char *minimal_xname(struct obj *); staticfn char *minimal_xname(struct obj *);
@@ -122,15 +122,16 @@ static const struct Jitem Japanese_items[] = {
staticfn char * staticfn char *
strprepend(char *s, const char *pref) strprepend(char *s, const char *pref)
{ {
char star_s = *s;
int i = (int) strlen(pref); int i = (int) strlen(pref);
if (i > PREFIX) { if (i > PREFIX) {
impossible("PREFIX too short (for %d).", i); impossible("PREFIX too short (for %d).", i);
return s; return s;
} }
s -= i; copynchars(s - i, pref, i + 1);
(void) strncpy(s, pref, i); /* do not copy trailing 0 */ *s = star_s;
return s; return s - i;
} }
/* manage a pool of BUFSZ buffers, so callers don't have to */ /* manage a pool of BUFSZ buffers, so callers don't have to */
@@ -2406,6 +2407,8 @@ ansimpleoname(struct obj *obj)
char *obufp, *simpleoname = simpleonames(obj); char *obufp, *simpleoname = simpleonames(obj);
int otyp = obj->otyp; int otyp = obj->otyp;
if (strlen(simpleoname) > BUFSZ - sizeof "the ")
simpleoname[sizeof "the "] = '\0';
/* prefix with "the" if a unique item, or a fake one imitating same, /* prefix with "the" if a unique item, or a fake one imitating same,
has been formatted with its actual name (we let minimal_xname() handle has been formatted with its actual name (we let minimal_xname() handle
any `known' and `dknown' checking necessary) */ any `known' and `dknown' checking necessary) */
@@ -2414,12 +2417,14 @@ ansimpleoname(struct obj *obj)
if (objects[otyp].oc_unique && OBJ_NAME(objects[otyp]) if (objects[otyp].oc_unique && OBJ_NAME(objects[otyp])
&& !strcmp(simpleoname, OBJ_NAME(objects[otyp]))) { && !strcmp(simpleoname, OBJ_NAME(objects[otyp]))) {
/* the() will allocate another obuf[]; we want to avoid using two */ /* the() will allocate another obuf[]; we want to avoid using two */
Strcpy(simpleoname, obufp = the(simpleoname)); obufp = the(simpleoname);
Strcpy(simpleoname, obufp);
releaseobuf(obufp); releaseobuf(obufp);
} else if (obj->quan == 1L) { } else if (obj->quan == 1L) {
/* simpleoname[] is singular if quan==1, plural otherwise; /* simpleoname[] is singular if quan==1, plural otherwise;
an() will allocate another obuf[]; we want to avoid using two */ an() will allocate another obuf[]; we want to avoid using two */
Strcpy(simpleoname, obufp = an(simpleoname)); obufp = an(simpleoname);
Strcpy(simpleoname, obufp);
releaseobuf(obufp); releaseobuf(obufp);
} }
return simpleoname; return simpleoname;
@@ -2432,7 +2437,8 @@ thesimpleoname(struct obj *obj)
char *obufp, *simpleoname = simpleonames(obj); char *obufp, *simpleoname = simpleonames(obj);
/* the() will allocate another obuf[]; we want to avoid using two */ /* the() will allocate another obuf[]; we want to avoid using two */
Strcpy(simpleoname, obufp = the(simpleoname)); obufp = the(simpleoname);
Strcpy(simpleoname, obufp);
releaseobuf(obufp); releaseobuf(obufp);
return simpleoname; return simpleoname;
} }
@@ -3513,7 +3519,7 @@ wizterrainwish(struct _readobjnam_data *d)
int trap; int trap;
unsigned oldtyp, ltyp; unsigned oldtyp, ltyp;
coordxy x = u.ux, y = u.uy; coordxy x = u.ux, y = u.uy;
char *bp = d->bp, *p = d->p; char *bp = d->bp, *p;
for (trap = NO_TRAP + 1; trap < TRAPNUM; trap++) { for (trap = NO_TRAP + 1; trap < TRAPNUM; trap++) {
struct trap *t; struct trap *t;
@@ -5586,6 +5592,7 @@ safe_qbuf(
lenlimit = QBUFSZ - 1; lenlimit = QBUFSZ - 1;
endp = qbuf + lenlimit; endp = qbuf + lenlimit;
assert(endp != NULL); /* workaround for static analyzer issue */
/* sanity check, aimed mainly at paniclog (it's conceivable for /* sanity check, aimed mainly at paniclog (it's conceivable for
the result of short_oname() to be shorter than the length of the result of short_oname() to be shorter than the length of
the last resort string, but we ignore that possibility here) */ the last resort string, but we ignore that possibility here) */
+4 -5
View File
@@ -1,4 +1,4 @@
/* NetHack 3.7 options.c $NHDT-Date: 1710792444 2024/03/18 20:07:24 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.723 $ */ /* NetHack 3.7 options.c $NHDT-Date: 1737556914 2025/01/22 06:41:54 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.753 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Michael Allison, 2008. */ /*-Copyright (c) Michael Allison, 2008. */
/* NetHack may be freely redistributed. See license for details. */ /* NetHack may be freely redistributed. See license for details. */
@@ -7768,7 +7768,7 @@ parse_role_opt(
char **opp) char **opp)
{ {
static char neg_opt[] = "!"; /* not 'const' but never modified */ static char neg_opt[] = "!"; /* not 'const' but never modified */
char *preval, *op = *opp; char *preval, *op;
int which = (optidx == opt_role) ? RS_ROLE int which = (optidx == opt_role) ? RS_ROLE
: (optidx == opt_race) ? RS_RACE : (optidx == opt_race) ? RS_RACE
: (optidx == opt_gender) ? RS_GENDER : (optidx == opt_gender) ? RS_GENDER
@@ -7855,7 +7855,7 @@ parse_role_opt(
if it's ok, replace it with canonical form */ if it's ok, replace it with canonical form */
saveoptstr(optidx, op); saveoptstr(optidx, op);
*opp = op; *opp = op;
ok = TRUE; /*ok = TRUE; // redundant*/
/* don't return yet; value might be a list that follows /* don't return yet; value might be a list that follows
this with something else which might make it invalid */ this with something else which might make it invalid */
} }
@@ -9095,7 +9095,6 @@ handle_add_list_remove(const char *optname, int numtotal)
}; };
int clr = NO_COLOR; int clr = NO_COLOR;
opt_idx = 0;
tmpwin = create_nhwindow(NHW_MENU); tmpwin = create_nhwindow(NHW_MENU);
start_menu(tmpwin, MENU_BEHAVE_STANDARD); start_menu(tmpwin, MENU_BEHAVE_STANDARD);
any = cg.zeroany; any = cg.zeroany;
@@ -9615,7 +9614,7 @@ next_opt(winid datawin, const char *str)
if (!*str) { if (!*str) {
s = eos(buf); s = eos(buf);
if (s > &buf[1] && s[-2] == ',') if (s > &buf[1] && s[-2] == ',')
Strcpy(s - 2, "."); /* replace last ", " */ s[-2] = '.', s[-1] = '\0'; /* replace ending ", " with "." */
i = COLNO; /* (greater than COLNO - 2) */ i = COLNO; /* (greater than COLNO - 2) */
} else { } else {
i = Strlen(buf) + Strlen(str) + 2; i = Strlen(buf) + Strlen(str) + 2;