From 61f969e88b3bb7b4f8187b917876b0e13247805f Mon Sep 17 00:00:00 2001 From: nhmall Date: Mon, 20 Jan 2025 14:36:28 -0500 Subject: [PATCH] follow-up for put_saddle_on_mon() Commit 1acc2727 helped ensure that the which_armor(mtmp, W_SADDLE) test at the top of put_saddle_on_mon() wouldn't lead to an obj leak. This commit covers off the adjacent can_saddle() test in put_saddle_on_mon(), because if that failed, it could also lead to a memory leak of the saddle obj passed by the caller. - have put_saddle_on_mon() create and use its own saddle obj if a NULL saddle obj is passed, instead of having to do that in the caller. - where an existing saddle obj needs to be passed from the caller, ensure that the caller has done its own can_saddle(mon) check prior to calling put_saddle_on_mon(), so that the can_saddle() test in put_saddle_on_mon() won't fail. - lastly, add an impossible() to put_saddle_on_mon() to catch a failure when a saddle obj is passed from the caller and either test has failed, just in case. That should not happen with any of the existing cases now, but it will provide some bullet-proofing for new code, new callers. --- include/extern.h | 2 +- src/dog.c | 11 ++++------- src/makemon.c | 6 +++--- src/read.c | 6 +++--- src/sp_lev.c | 2 +- src/steed.c | 14 +++++++++++++- 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/include/extern.h b/include/extern.h index 6db547e30..d14e6d30c 100644 --- a/include/extern.h +++ b/include/extern.h @@ -3056,7 +3056,7 @@ extern struct obj *findgold(struct obj *) NO_NNARGS; extern void rider_cant_reach(void); extern boolean can_saddle(struct monst *) NONNULLARG1; extern int use_saddle(struct obj *) NONNULLARG1; -extern void put_saddle_on_mon(struct obj *, struct monst *) NONNULLARG12; +extern void put_saddle_on_mon(struct obj *, struct monst *) NONNULLARG2; extern boolean can_ride(struct monst *) NONNULLARG1; extern int doride(void); extern boolean mount_steed(struct monst *, boolean) NO_NNARGS; diff --git a/src/dog.c b/src/dog.c index 7fffb6da9..07336ef71 100644 --- a/src/dog.c +++ b/src/dog.c @@ -191,7 +191,6 @@ struct monst * makedog(void) { struct monst *mtmp; - struct obj *otmp; const char *petname; int pettype; @@ -231,12 +230,10 @@ makedog(void) svc.context.startingpet_mid = mtmp->m_id; if (!u.uroleplay.pauper) { /* initial horses start wearing a saddle (pauper hero excluded) */ - if (pettype == PM_PONY - && (otmp = mksobj(SADDLE, TRUE, FALSE)) != 0) { - /* pseudo initial inventory; saddle is not actually in hero's - * invent so assume that update_inventory() isn't needed */ - fully_identify_obj(otmp); - put_saddle_on_mon(otmp, mtmp); + if (pettype == PM_PONY) { + /* NULL obj arg means put_saddle_on_mon() + * will carry out the saddle creation */ + put_saddle_on_mon((struct obj *) 0, mtmp); } } } else { diff --git a/src/makemon.c b/src/makemon.c index 9ff2c2da5..a7e52bf47 100644 --- a/src/makemon.c +++ b/src/makemon.c @@ -1439,9 +1439,9 @@ makemon( if (!rn2(100) && is_domestic(ptr) && can_saddle(mtmp) && !which_armor(mtmp, W_SADDLE)) { - struct obj *otmp = mksobj(SADDLE, TRUE, FALSE); - - put_saddle_on_mon(otmp, mtmp); + /* NULL obj arg means put_saddle_on_mon() + * will create the saddle itself */ + put_saddle_on_mon((struct obj *) 0, mtmp); } } else { diff --git a/src/read.c b/src/read.c index 831d85118..b8cbdc881 100644 --- a/src/read.c +++ b/src/read.c @@ -3237,9 +3237,9 @@ create_particular_creation( set_malign(mtmp); } if (d->saddled && can_saddle(mtmp) && !which_armor(mtmp, W_SADDLE)) { - struct obj *otmp = mksobj(SADDLE, TRUE, FALSE); - - put_saddle_on_mon(otmp, mtmp); + /* NULL obj arg means put_saddle_on_mon() + * will create the saddle itself */ + put_saddle_on_mon((struct obj *) 0, mtmp); } if (d->hidden && ((is_hider(mtmp->data) && mtmp->data->mlet != S_MIMIC) diff --git a/src/sp_lev.c b/src/sp_lev.c index f18f795e9..62f83909c 100644 --- a/src/sp_lev.c +++ b/src/sp_lev.c @@ -2307,7 +2307,7 @@ create_object(object *o, struct mkroom *croom) ; /* ['otmp' remains on floor] */ } else { remove_object(otmp); - if (otmp->otyp == SADDLE) + if (otmp->otyp == SADDLE && can_saddle(invent_carrying_monster)) put_saddle_on_mon(otmp, invent_carrying_monster); else (void) mpickobj(invent_carrying_monster, otmp); diff --git a/src/steed.c b/src/steed.c index 84a6e9287..48d537b99 100644 --- a/src/steed.c +++ b/src/steed.c @@ -131,6 +131,7 @@ use_saddle(struct obj *otmp) if (otmp->owornmask) remove_worn_item(otmp, FALSE); freeinv(otmp); + /* !can_saddle(mtmp) already eliminated above */ put_saddle_on_mon(otmp, mtmp); } else pline("%s resists!", Monnam(mtmp)); @@ -140,8 +141,19 @@ use_saddle(struct obj *otmp) void put_saddle_on_mon(struct obj *saddle, struct monst *mtmp) { - if (!can_saddle(mtmp) || which_armor(mtmp, W_SADDLE)) + if (!can_saddle(mtmp) || which_armor(mtmp, W_SADDLE)) { + if (saddle) + impossible("put_saddle_on_mon: saddle obj could get orphaned"); return; + } + if (!saddle) { + if ((saddle = mksobj(SADDLE, TRUE, FALSE)) != 0) { + fully_identify_obj(saddle); + /* mpickobj can later override identification if out-of-view */ + } else { + return; + } + } if (mpickobj(mtmp, saddle)) panic("merged saddle?"); mtmp->misc_worn_check |= W_SADDLE;