From 3109e706e9e69267a8429f979f0f3aa3373d4363 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 21 Jan 2025 22:42:23 -0800 Subject: [PATCH] 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. --- src/objnam.c | 27 +++++++++++++++++---------- src/options.c | 9 ++++----- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/objnam.c b/src/objnam.c index 8340f752b..d71dd5897 100644 --- a/src/objnam.c +++ b/src/objnam.c @@ -32,9 +32,9 @@ struct _readobjnam_data { char fruitbuf[BUFSZ]; }; -staticfn char *strprepend(char *, const char *); -staticfn char *nextobuf(void); -staticfn void releaseobuf(char *); +staticfn char *strprepend(char *, const char *) NONNULL NONNULLARG1; +staticfn char *nextobuf(void) NONNULL; +staticfn void releaseobuf(char *) NONNULLARG1; staticfn void xcalled(char *, int, const char *, const char *); staticfn char *xname_flags(struct obj *, unsigned); staticfn char *minimal_xname(struct obj *); @@ -122,15 +122,16 @@ static const struct Jitem Japanese_items[] = { staticfn char * strprepend(char *s, const char *pref) { + char star_s = *s; int i = (int) strlen(pref); if (i > PREFIX) { impossible("PREFIX too short (for %d).", i); return s; } - s -= i; - (void) strncpy(s, pref, i); /* do not copy trailing 0 */ - return s; + copynchars(s - i, pref, i + 1); + *s = star_s; + return s - i; } /* 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); 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, has been formatted with its actual name (we let minimal_xname() handle any `known' and `dknown' checking necessary) */ @@ -2414,12 +2417,14 @@ ansimpleoname(struct obj *obj) if (objects[otyp].oc_unique && OBJ_NAME(objects[otyp]) && !strcmp(simpleoname, OBJ_NAME(objects[otyp]))) { /* the() will allocate another obuf[]; we want to avoid using two */ - Strcpy(simpleoname, obufp = the(simpleoname)); + obufp = the(simpleoname); + Strcpy(simpleoname, obufp); releaseobuf(obufp); } else if (obj->quan == 1L) { /* simpleoname[] is singular if quan==1, plural otherwise; an() will allocate another obuf[]; we want to avoid using two */ - Strcpy(simpleoname, obufp = an(simpleoname)); + obufp = an(simpleoname); + Strcpy(simpleoname, obufp); releaseobuf(obufp); } return simpleoname; @@ -2432,7 +2437,8 @@ thesimpleoname(struct obj *obj) char *obufp, *simpleoname = simpleonames(obj); /* the() will allocate another obuf[]; we want to avoid using two */ - Strcpy(simpleoname, obufp = the(simpleoname)); + obufp = the(simpleoname); + Strcpy(simpleoname, obufp); releaseobuf(obufp); return simpleoname; } @@ -3513,7 +3519,7 @@ wizterrainwish(struct _readobjnam_data *d) int trap; unsigned oldtyp, ltyp; 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++) { struct trap *t; @@ -5586,6 +5592,7 @@ safe_qbuf( lenlimit = QBUFSZ - 1; endp = qbuf + lenlimit; + assert(endp != NULL); /* workaround for static analyzer issue */ /* sanity check, aimed mainly at paniclog (it's conceivable for the result of short_oname() to be shorter than the length of the last resort string, but we ignore that possibility here) */ diff --git a/src/options.c b/src/options.c index 805efe3d4..468f8be00 100644 --- a/src/options.c +++ b/src/options.c @@ -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) Michael Allison, 2008. */ /* NetHack may be freely redistributed. See license for details. */ @@ -7768,7 +7768,7 @@ parse_role_opt( char **opp) { static char neg_opt[] = "!"; /* not 'const' but never modified */ - char *preval, *op = *opp; + char *preval, *op; int which = (optidx == opt_role) ? RS_ROLE : (optidx == opt_race) ? RS_RACE : (optidx == opt_gender) ? RS_GENDER @@ -7855,7 +7855,7 @@ parse_role_opt( if it's ok, replace it with canonical form */ saveoptstr(optidx, op); *opp = op; - ok = TRUE; + /*ok = TRUE; // redundant*/ /* don't return yet; value might be a list that follows 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; - opt_idx = 0; tmpwin = create_nhwindow(NHW_MENU); start_menu(tmpwin, MENU_BEHAVE_STANDARD); any = cg.zeroany; @@ -9615,7 +9614,7 @@ next_opt(winid datawin, const char *str) if (!*str) { s = eos(buf); 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) */ } else { i = Strlen(buf) + Strlen(str) + 2;