From 4c75260097135edb171501942e451474fce66c50 Mon Sep 17 00:00:00 2001 From: Dragorn421 Date: Sun, 12 Nov 2023 22:59:52 +0100 Subject: [PATCH] Fix misc 21 (#1573) * Make `sNew` in (unused) `code_800FC620.c` a string It is passed as a filename to `__osMallocDebug` so should be a nul-terminated string, not a char[3] missing an explicit \0 * Fix gcc warning in `JpegDecoder_ParseNextSymbol` about SLL on negative value -1U is an unsigned value, aka 0xFFFFFFFF I keep -1 because it seems that's what a jpeg standard has too References: https://stackoverflow.com/questions/40508958/shifting-a-negative-signed-value-is-undefined https://www.w3.org/Graphics/JPEG/itu-t81.pdf (page 105, figure F.12) * Small cleanup * Fix few mistakes (thanks gcc warnings) * Add `@bug` in file select settings draw code, using the wrong array * format * format main * rename arg for a happy formatter * Move important function call out of a printf --- src/code/code_800FC620.c | 4 +-- src/code/jpegdecoder.c | 2 +- src/code/z_play.c | 5 +++- src/code/z_player_lib.c | 2 +- src/libultra/libc/ll.c | 4 +-- src/libultra/libc/xlitob.c | 8 ++--- src/libultra/libc/xprintf.c | 30 +++++++++---------- .../ovl_Bg_Jya_Bigmirror/z_bg_jya_bigmirror.c | 2 +- .../actors/ovl_Boss_Ganon2/z_boss_ganon2.c | 4 +-- .../actors/ovl_En_Bigokuta/z_en_bigokuta.c | 2 +- src/overlays/actors/ovl_En_Fr/z_en_fr.c | 10 +++---- src/overlays/actors/ovl_En_Gs/z_en_gs.c | 4 +-- .../ovl_En_Po_Sisters/z_en_po_sisters.c | 4 +-- .../ovl_file_choose/z_file_nameset_PAL.c | 5 ++++ 14 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/code/code_800FC620.c b/src/code/code_800FC620.c index d31d028a9b..deca11a4fa 100644 --- a/src/code/code_800FC620.c +++ b/src/code/code_800FC620.c @@ -13,7 +13,7 @@ typedef struct InitFunc { // .data void* sInitFuncs = NULL; -char sNew[] = { 'n', 'e', 'w' }; +char sNew[] = "new"; char D_80134488[0x18] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7F, 0x80, 0x00, 0x00, @@ -100,7 +100,7 @@ void func_800FCB34(void) { initFunc = (InitFunc*)((s32)initFunc + nextOffset); if (initFunc->func != NULL) { - (*initFunc->func)(); + initFunc->func(); } nextOffset = initFunc->nextOffset; diff --git a/src/code/jpegdecoder.c b/src/code/jpegdecoder.c index 3301817d0f..c0c772abd6 100644 --- a/src/code/jpegdecoder.c +++ b/src/code/jpegdecoder.c @@ -154,7 +154,7 @@ s32 JpegDecoder_ParseNextSymbol(JpegHuffmanTable* hTable, s16* outCoeff, s8* out if (sym) { *outCoeff = JpegDecoder_ReadBits(sym); if (*outCoeff < (1 << (sym - 1))) { - *outCoeff += (-1 << sym) + 1; + *outCoeff += (-1U << sym) + 1; } } diff --git a/src/code/z_play.c b/src/code/z_play.c index 58fa9f726c..b71a60564b 100644 --- a/src/code/z_play.c +++ b/src/code/z_play.c @@ -1447,6 +1447,7 @@ void Play_InitScene(PlayState* this, s32 spawn) { void Play_SpawnScene(PlayState* this, s32 sceneId, s32 spawn) { SceneTableEntry* scene = &gSceneTable[sceneId]; + u32 size; scene->unk_13 = 0; this->loadedScene = scene; @@ -1463,7 +1464,9 @@ void Play_SpawnScene(PlayState* this, s32 sceneId, s32 spawn) { Play_InitScene(this, spawn); - osSyncPrintf("ROOM SIZE=%fK\n", func_80096FE8(this, &this->roomCtx) / 1024.0f); + size = func_80096FE8(this, &this->roomCtx); + + osSyncPrintf("ROOM SIZE=%fK\n", size / 1024.0f); } void Play_GetScreenPos(PlayState* this, Vec3f* src, Vec3f* dest) { diff --git a/src/code/z_player_lib.c b/src/code/z_player_lib.c index 39a56c5456..6c12331d56 100644 --- a/src/code/z_player_lib.c +++ b/src/code/z_player_lib.c @@ -1061,7 +1061,7 @@ s32 Player_OverrideLimbDrawGameplayCommon(PlayState* play, s32 limbIndex, Gfx** // Note: The increment would not be done for the root limb, even if it had a non-NULL `dList`. // So if the root limb had a non-NULL `dList` (which is not the case in vanilla), // an out-of-bounds write to `bodyPartsPos` would occur. - sCurBodyPartPos = &this->bodyPartsPos[-1]; + sCurBodyPartPos = &this->bodyPartsPos[0] - 1; if (!LINK_IS_ADULT) { if (!(this->skelAnime.moveFlags & ANIM_FLAG_PLAYER_2) || (this->skelAnime.moveFlags & ANIM_FLAG_0)) { diff --git a/src/libultra/libc/ll.c b/src/libultra/libc/ll.c index df56fb3f33..759d872734 100644 --- a/src/libultra/libc/ll.c +++ b/src/libultra/libc/ll.c @@ -28,8 +28,8 @@ long long __ll_mul(long long left, long long right) { return left * right; } -void __ull_divremi(unsigned long long int* quotient, unsigned long long int* remainder, - unsigned long long dividend, unsigned short divisor) { +void __ull_divremi(unsigned long long int* quotient, unsigned long long int* remainder, unsigned long long dividend, + unsigned short divisor) { *quotient = dividend / divisor; *remainder = dividend % divisor; } diff --git a/src/libultra/libc/xlitob.c b/src/libultra/libc/xlitob.c index 32e769b160..2a33062265 100644 --- a/src/libultra/libc/xlitob.c +++ b/src/libultra/libc/xlitob.c @@ -7,24 +7,24 @@ static char ldigs[] = "0123456789abcdef"; static char udigs[] = "0123456789ABCDEF"; -void _Litob(_Pft* args, char type) { +void _Litob(_Pft* args, char code) { char buff[BUFF_LEN]; const char* digs; int base; int i; unsigned long long num; - if (type == 'X') { + if (code == 'X') { digs = udigs; } else { digs = ldigs; } - base = (type == 'o') ? 8 : ((type != 'x' && type != 'X') ? 10 : 16); + base = (code == 'o') ? 8 : ((code != 'x' && code != 'X') ? 10 : 16); i = BUFF_LEN; num = args->v.ll; - if ((type == 'd' || type == 'i') && args->v.ll < 0) { + if ((code == 'd' || code == 'i') && args->v.ll < 0) { num = -num; } diff --git a/src/libultra/libc/xprintf.c b/src/libultra/libc/xprintf.c index a838c7173f..9470e141fe 100644 --- a/src/libultra/libc/xprintf.c +++ b/src/libultra/libc/xprintf.c @@ -8,9 +8,9 @@ #define isdigit(x) (((x) >= '0' && (x) <= '9')) #define LDSIGN(x) (((unsigned short*)&(x))[0] & 0x8000) -#define ATOI(i, a) \ - for (i = 0; isdigit(*a); a++) \ - if (i < 999) \ +#define ATOI(i, a) \ + for (i = 0; isdigit(*a); a++) \ + if (i < 999) \ i = *a + i * 10 - '0'; #define PUT(fmt, _size) \ @@ -24,17 +24,17 @@ #define MAX_PAD ((int)sizeof(spaces) - 1) -#define PAD(src, m) \ - if (m > 0) { \ - int i; \ - int j; \ - for (j = m; j > 0; j -= i) { \ - if ((unsigned)j > MAX_PAD) \ - i = MAX_PAD; \ - else \ - i = j; \ - PUT(src, i); \ - } \ +#define PAD(src, m) \ + if (m > 0) { \ + int i; \ + int j; \ + for (j = m; j > 0; j -= i) { \ + if ((unsigned)j > MAX_PAD) \ + i = MAX_PAD; \ + else \ + i = j; \ + PUT(src, i); \ + } \ } char spaces[] = " "; @@ -69,7 +69,7 @@ int _Printf(PrintCallback pfn, void* arg, const char* fmt, va_list ap) { for (x.flags = 0; (t = strchr(fchar, *s)) != NULL; s++) { x.flags |= fbit[t - fchar]; } - + if (*s == '*') { x.width = va_arg(ap, int); if (x.width < 0) { diff --git a/src/overlays/actors/ovl_Bg_Jya_Bigmirror/z_bg_jya_bigmirror.c b/src/overlays/actors/ovl_Bg_Jya_Bigmirror/z_bg_jya_bigmirror.c index ee5050c95c..fb70503b33 100644 --- a/src/overlays/actors/ovl_Bg_Jya_Bigmirror/z_bg_jya_bigmirror.c +++ b/src/overlays/actors/ovl_Bg_Jya_Bigmirror/z_bg_jya_bigmirror.c @@ -84,7 +84,7 @@ void BgJyaBigmirror_HandleCobra(Actor* thisx, PlayState* play) { curSpawnData->pos.z, 0, curCobraInfo->rotY, 0, curSpawnData->params); this->actor.child = NULL; - if (&curCobraInfo->cobra->dyna.actor == NULL) { + if (curCobraInfo->cobra == NULL) { // "Cobra generation failed" osSyncPrintf("Error : コブラ発生失敗 (%s %d)\n", "../z_bg_jya_bigmirror.c", 221); } diff --git a/src/overlays/actors/ovl_Boss_Ganon2/z_boss_ganon2.c b/src/overlays/actors/ovl_Boss_Ganon2/z_boss_ganon2.c index 26b0e741d2..1920aded17 100644 --- a/src/overlays/actors/ovl_Boss_Ganon2/z_boss_ganon2.c +++ b/src/overlays/actors/ovl_Boss_Ganon2/z_boss_ganon2.c @@ -2522,8 +2522,8 @@ s32 BossGanon2_OverrideLimbDraw(PlayState* play, s32 limbIndex, Gfx** dList, Vec } if (limbIndex >= GANON_LIMB_TAIL1) { - rot->x += this->unk_2F4[limbIndex] + this->unk_346; - rot->y += this->unk_2FE[limbIndex]; + rot->x += this->unk_348[limbIndex - GANON_LIMB_TAIL1] + this->unk_346; + rot->y += this->unk_352[limbIndex - GANON_LIMB_TAIL1]; if (this->unk_342 & 1) { gDPSetEnvColor(POLY_OPA_DISP++, 255, 0, 0, 255); diff --git a/src/overlays/actors/ovl_En_Bigokuta/z_en_bigokuta.c b/src/overlays/actors/ovl_En_Bigokuta/z_en_bigokuta.c index 3df1f4505b..19e521a872 100644 --- a/src/overlays/actors/ovl_En_Bigokuta/z_en_bigokuta.c +++ b/src/overlays/actors/ovl_En_Bigokuta/z_en_bigokuta.c @@ -152,7 +152,7 @@ static InitChainEntry sInitChain[] = { }; // possibly color data -static s32 sUnused[] = { 0xFFFFFFFF, 0x969696FF }; +static u32 sUnused[] = { 0xFFFFFFFF, 0x969696FF }; void EnBigokuta_Init(Actor* thisx, PlayState* play) { EnBigokuta* this = (EnBigokuta*)thisx; diff --git a/src/overlays/actors/ovl_En_Fr/z_en_fr.c b/src/overlays/actors/ovl_En_Fr/z_en_fr.c index a4b083cf2c..0b3bc3968a 100644 --- a/src/overlays/actors/ovl_En_Fr/z_en_fr.c +++ b/src/overlays/actors/ovl_En_Fr/z_en_fr.c @@ -776,19 +776,19 @@ void EnFr_CheckOcarinaInputFrogSong(u8 ocarinaNote) { s32 frogIndex; switch (ocarinaNote) { - case 0: + case OCARINA_BTN_A: frogIndexButterfly = FROG_BLUE; break; - case 1: + case OCARINA_BTN_C_DOWN: frogIndexButterfly = FROG_YELLOW; break; - case 2: + case OCARINA_BTN_C_RIGHT: frogIndexButterfly = FROG_RED; break; - case 3: + case OCARINA_BTN_C_LEFT: frogIndexButterfly = FROG_PURPLE; break; - case 4: + case OCARINA_BTN_C_UP: frogIndexButterfly = FROG_WHITE; } // Turn on or off butterfly above frog diff --git a/src/overlays/actors/ovl_En_Gs/z_en_gs.c b/src/overlays/actors/ovl_En_Gs/z_en_gs.c index 6500bb69f9..3bc1c52911 100644 --- a/src/overlays/actors/ovl_En_Gs/z_en_gs.c +++ b/src/overlays/actors/ovl_En_Gs/z_en_gs.c @@ -435,7 +435,7 @@ void func_80A4F13C(EnGs* this, PlayState* play) { tmp = this->unk_1A0[0].y; if (tmp > 0) { - tmp += 0xFFFF0000; + tmp -= 0x10000; } this->unk_1E8 = tmp; @@ -446,7 +446,7 @@ void func_80A4F13C(EnGs* this, PlayState* play) { if (this->unk_19F == 5) { tmp = this->unk_1A0[0].y; if (tmp > 0) { - tmp += 0xFFFF0001; + tmp -= 0xFFFF; } this->unk_1E8 = tmp; tmpf1 = Math_SmoothStepToF(&this->unk_1E8, this->unk_1EC, 0.8f, 3640.0f, 0.001f); diff --git a/src/overlays/actors/ovl_En_Po_Sisters/z_en_po_sisters.c b/src/overlays/actors/ovl_En_Po_Sisters/z_en_po_sisters.c index ff1029c3d1..799dc2a83e 100644 --- a/src/overlays/actors/ovl_En_Po_Sisters/z_en_po_sisters.c +++ b/src/overlays/actors/ovl_En_Po_Sisters/z_en_po_sisters.c @@ -1275,9 +1275,9 @@ s32 EnPoSisters_OverrideLimbDraw(PlayState* play, s32 limbIndex, Gfx** dList, Ve if (limbIndex == 1 && (this->unk_199 & 0x40)) { if (this->unk_19A >= 284) { - rot->x += (this->unk_19A * 0x1000) + 0xFFEE4000; + rot->x += (this->unk_19A * 0x1000) - 0x11C000; } else { - rot->x += (this->unk_19A * 0x1000) + 0xFFFF1000; + rot->x += (this->unk_19A * 0x1000) - 0xF000; } } if (this->unk_22E.a == 0 || limbIndex == 8 || (this->actionFunc == func_80ADAFC0 && this->unk_19A >= 8)) { diff --git a/src/overlays/gamestates/ovl_file_choose/z_file_nameset_PAL.c b/src/overlays/gamestates/ovl_file_choose/z_file_nameset_PAL.c index 67cc0a0748..101c79b8bd 100644 --- a/src/overlays/gamestates/ovl_file_choose/z_file_nameset_PAL.c +++ b/src/overlays/gamestates/ovl_file_choose/z_file_nameset_PAL.c @@ -917,6 +917,8 @@ void FileSelect_DrawOptionsImpl(GameState* thisx) { gDPSetEnvColor(POLY_OPA_DISP++, 0, 0, 0, 255); } + //! @bug Mistakenly using sOptionsMenuHeaders instead of sOptionsMenuSettings for the height. + //! This works out anyway because all heights are 16. gDPLoadTextureBlock(POLY_OPA_DISP++, sOptionsMenuSettings[i].texture[gSaveContext.language], G_IM_FMT_IA, G_IM_SIZ_8b, sOptionsMenuSettings[i].width[gSaveContext.language], sOptionsMenuHeaders[i].height, 0, G_TX_NOMIRROR | G_TX_WRAP, G_TX_NOMIRROR | G_TX_WRAP, @@ -941,6 +943,9 @@ void FileSelect_DrawOptionsImpl(GameState* thisx) { gDPSetEnvColor(POLY_OPA_DISP++, 0, 0, 0, 255); } + //! @bug Mistakenly using sOptionsMenuHeaders instead of sOptionsMenuSettings for the height. + //! This is also an OOB read that happens to access the height of the first two elements in + //! sOptionsMenuSettings, and since all heights are 16, it works out anyway. gDPLoadTextureBlock(POLY_OPA_DISP++, sOptionsMenuSettings[i].texture[gSaveContext.language], G_IM_FMT_IA, G_IM_SIZ_8b, sOptionsMenuSettings[i].width[gSaveContext.language], sOptionsMenuHeaders[i].height, 0, G_TX_NOMIRROR | G_TX_WRAP, G_TX_NOMIRROR | G_TX_WRAP,