From f84ebf6bff8e2ba761e4dd038e0d9f3f3de61419 Mon Sep 17 00:00:00 2001 From: Henrik Rydgard Date: Sun, 14 Sep 2014 00:14:11 +0200 Subject: [PATCH] sprintf->snprintf, fix some too short buffers --- Core/Config.cpp | 4 +-- Core/Debugger/DisassemblyManager.cpp | 13 +++++----- Core/HLE/sceKernelMemory.cpp | 4 ++- Core/MIPS/ARM/ArmJit.cpp | 2 +- Core/SaveState.cpp | 10 ++++---- UI/DevScreens.cpp | 5 ++-- UI/MiscScreens.cpp | 4 +-- UI/NativeApp.cpp | 6 ++--- Windows/Debugger/BreakpointWindow.cpp | 20 +++++++++------ Windows/Debugger/CtrlDisAsmView.cpp | 35 ++++++++++++--------------- Windows/Debugger/CtrlDisAsmView.h | 2 +- Windows/Debugger/Debugger_Disasm.cpp | 7 +++--- Windows/Debugger/Debugger_Lists.cpp | 4 +-- Windows/Debugger/DumpMemoryWindow.cpp | 7 +++--- ext/disarm.cpp | 2 +- ext/disarm.h | 2 +- native | 2 +- unittest/UnitTest.cpp | 2 +- 18 files changed, 66 insertions(+), 65 deletions(-) diff --git a/Core/Config.cpp b/Core/Config.cpp index e090cc7092..23a3d43b8a 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -744,7 +744,7 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename) { char keyName[64]; std::string fileName; - sprintf(keyName, "FileName%d", i); + snprintf(keyName, sizeof(keyName), "FileName%d", i); if (recent->Get(keyName, &fileName, "") && !fileName.empty()) { recentIsos.push_back(fileName); } @@ -853,7 +853,7 @@ void Config::Save() { for (int i = 0; i < iMaxRecent; i++) { char keyName[64]; - sprintf(keyName,"FileName%d",i); + snprintf(keyName, sizeof(keyName), "FileName%d", i); if (i < (int)recentIsos.size()) { recent->Set(keyName, recentIsos[i]); } else { diff --git a/Core/Debugger/DisassemblyManager.cpp b/Core/Debugger/DisassemblyManager.cpp index 0ba3cc1467..874207b2f6 100644 --- a/Core/Debugger/DisassemblyManager.cpp +++ b/Core/Debugger/DisassemblyManager.cpp @@ -959,7 +959,7 @@ void DisassemblyData::createLines() } else { while (pos < end) { - char buffer[64]; + char buffer[256]; u32 value; u32 currentPos = pos; @@ -968,12 +968,12 @@ void DisassemblyData::createLines() { case DATATYPE_BYTE: value = Memory::Read_U8(pos); - sprintf(buffer,"0x%02X",value); + snprintf(buffer, sizeof(buffer), "0x%02X", value); pos++; break; case DATATYPE_HALFWORD: value = Memory::Read_U16(pos); - sprintf(buffer,"0x%04X",value); + snprintf(buffer, sizeof(buffer), "0x%04X", value); pos += 2; break; case DATATYPE_WORD: @@ -981,9 +981,9 @@ void DisassemblyData::createLines() value = Memory::Read_U32(pos); const std::string label = symbolMap.GetLabelString(value); if (!label.empty()) - sprintf(buffer,"%s",label.c_str()); + snprintf(buffer, sizeof(buffer), "%s", label.c_str()); else - sprintf(buffer,"0x%08X",value); + snprintf(buffer, sizeof(buffer), "0x%08X", value); pos += 4; } break; @@ -1007,8 +1007,7 @@ void DisassemblyData::createLines() currentLine += buffer; } - if (currentLine.size() != 0) - { + if (currentLine.size() != 0) { DataEntry entry = {currentLine,pos-currentLineStart,lineCount++}; lines[currentLineStart] = entry; lineAddresses.push_back(currentLineStart); diff --git a/Core/HLE/sceKernelMemory.cpp b/Core/HLE/sceKernelMemory.cpp index 7d848ceb35..5389873548 100644 --- a/Core/HLE/sceKernelMemory.cpp +++ b/Core/HLE/sceKernelMemory.cpp @@ -20,6 +20,8 @@ #include #include +#include "base/compat.h" + #include "Common/ChunkFile.h" #include "Core/HLE/HLE.h" #include "Core/HLE/FunctionWrappers.h" @@ -899,7 +901,7 @@ public: void GetQuickInfo(char *ptr, int size) { int sz = alloc->GetBlockSizeFromAddress(address); - sprintf(ptr, "MemPart: %08x - %08x size: %08x", address, address + sz, sz); + snprintf(ptr, size, "MemPart: %08x - %08x size: %08x", address, address + sz, sz); } static u32 GetMissingErrorCode() { return SCE_KERNEL_ERROR_UNKNOWN_UID; } static int GetStaticIDType() { return PPSSPP_KERNEL_TMID_PMB; } diff --git a/Core/MIPS/ARM/ArmJit.cpp b/Core/MIPS/ARM/ArmJit.cpp index 212ad15d62..d6c135fbf4 100644 --- a/Core/MIPS/ARM/ArmJit.cpp +++ b/Core/MIPS/ARM/ArmJit.cpp @@ -53,7 +53,7 @@ void DisassembleArm(const u8 *data, int size) { continue; } } - ArmDis((u32)codePtr, inst, temp); + ArmDis((u32)codePtr, inst, temp, sizeof(temp), true); ILOG("A: %s", temp); } } diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 33c438b285..d284d0d758 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -289,11 +289,11 @@ namespace SaveState std::string GenerateSaveSlotFilename(int slot, const char *extension) { char discID[256]; - char temp[256]; - sprintf(discID, "%s_%s", + char temp[2048]; + snprintf(discID, sizeof(discID), "%s_%s", g_paramSFO.GetValueString("DISC_ID").c_str(), g_paramSFO.GetValueString("DISC_VERSION").c_str()); - sprintf(temp, "ms0:/PSP/PPSSPP_STATE/%s_%i.%s", discID, slot, extension); + snprintf(temp, sizeof(temp), "ms0:/PSP/PPSSPP_STATE/%s_%i.%s", discID, slot, extension); std::string hostPath; if (pspFileSystem.GetHostPath(std::string(temp), hostPath)) { return hostPath; @@ -306,8 +306,8 @@ namespace SaveState { I18NCategory *sy = GetI18NCategory("System"); g_Config.iCurrentStateSlot = (g_Config.iCurrentStateSlot + 1) % SaveState::SAVESTATESLOTS; - char msg[30]; - sprintf(msg, "%s: %d", sy->T("Savestate Slot"), g_Config.iCurrentStateSlot + 1); + char msg[128]; + snprintf(msg, sizeof(msg), "%s: %d", sy->T("Savestate Slot"), g_Config.iCurrentStateSlot + 1); osm.Show(msg); } diff --git a/UI/DevScreens.cpp b/UI/DevScreens.cpp index 161c100ec6..8313663159 100644 --- a/UI/DevScreens.cpp +++ b/UI/DevScreens.cpp @@ -17,6 +17,7 @@ #include +#include "base/compat.h" #include "gfx_es2/gl_state.h" #include "i18n/i18n.h" #include "ui/ui_context.h" @@ -447,7 +448,7 @@ std::vector DisassembleArm2(const u8 *data, int size) { continue; } } - ArmDis((u32)(intptr_t)codePtr, inst, temp, false); + ArmDis((u32)(intptr_t)codePtr, inst, temp, sizeof(temp), false); std::string buf = temp; lines.push_back(buf); } @@ -471,7 +472,7 @@ void JitCompareScreen::UpdateDisasm() { JitBlock *block = blockCache->GetBlock(currentBlock_); char temp[256]; - sprintf(temp, "%i/%i\n%08x", currentBlock_, blockCache->GetNumBlocks(), block->originalAddress); + snprintf(temp, sizeof(temp), "%i/%i\n%08x", currentBlock_, blockCache->GetNumBlocks(), block->originalAddress); blockName_->SetText(temp); // Alright. First generate the MIPS disassembly. diff --git a/UI/MiscScreens.cpp b/UI/MiscScreens.cpp index 431eba91c3..b4ba683a03 100644 --- a/UI/MiscScreens.cpp +++ b/UI/MiscScreens.cpp @@ -415,7 +415,7 @@ void LogoScreen::render() { I18NCategory *c = GetI18NCategory("PSPCredits"); char temp[256]; - sprintf(temp, "%s Henrik Rydg\xc3\xa5rd", c->T("created", "Created by")); + snprintf(temp, sizeof(temp), "%s Henrik Rydg\xc3\xa5rd", c->T("created", "Created by")); #ifdef GOLD dc.Draw()->DrawImage(I_ICONGOLD, bounds.centerX() - 120, bounds.centerY() - 30, 1.2f, colorAlpha(0xFFFFFFFF, alphaText), ALIGN_CENTER); #else @@ -626,7 +626,7 @@ void CreditsScreen::render() { // TODO: This is kinda ugly, done on every frame... char temp[256]; - sprintf(temp, "PPSSPP %s", PPSSPP_GIT_VERSION); + snprintf(temp, sizeof(temp), "PPSSPP %s", PPSSPP_GIT_VERSION); credits[0] = (const char *)temp; UIContext &dc = *screenManager()->getUIContext(); diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index a88dcb19d7..3efbdbaf52 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -602,12 +602,12 @@ void TakeScreenshot() { gameId = "MENU"; } - char filename[256]; + char filename[2048]; while (i < 10000){ if (g_Config.bScreenshotsAsPNG) - sprintf(filename, "%s/PSP/SCREENSHOT/%s_%05d.png", g_Config.memCardDirectory.c_str(), gameId.c_str(), i); + snprintf(filename, sizeof(filename), "%s/PSP/SCREENSHOT/%s_%05d.png", g_Config.memCardDirectory.c_str(), gameId.c_str(), i); else - sprintf(filename, "%s/PSP/SCREENSHOT/%s_%05d.jpg", g_Config.memCardDirectory.c_str(), gameId.c_str(), i); + snprintf(filename, sizeof(filename), "%s/PSP/SCREENSHOT/%s_%05d.jpg", g_Config.memCardDirectory.c_str(), gameId.c_str(), i); FileInfo info; if (!getFileInfo(filename, &info)) break; diff --git a/Windows/Debugger/BreakpointWindow.cpp b/Windows/Debugger/BreakpointWindow.cpp index fd76d211b1..6f1e920a15 100644 --- a/Windows/Debugger/BreakpointWindow.cpp +++ b/Windows/Debugger/BreakpointWindow.cpp @@ -1,6 +1,10 @@ +#include + +#include "base/compat.h" + #include "BreakpointWindow.h" #include "../resource.h" -#include + BreakpointWindow* BreakpointWindow::bp; @@ -29,11 +33,11 @@ INT_PTR CALLBACK BreakpointWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam, if (bp->address != -1) { - sprintf(str,"0x%08X",bp->address); + snprintf(str, sizeof(str), "0x%08X", bp->address); SetWindowTextA(GetDlgItem(hwnd,IDC_BREAKPOINT_ADDRESS),str); } - sprintf(str,"0x%08X",bp->size); + snprintf(str, sizeof(str), "0x%08X", bp->size); SetWindowTextA(GetDlgItem(hwnd,IDC_BREAKPOINT_SIZE),str); SetWindowTextA(GetDlgItem(hwnd,IDC_BREAKPOINT_CONDITION),bp->condition); @@ -123,14 +127,14 @@ bool BreakpointWindow::fetchDialogData(HWND hwnd) GetWindowTextA(GetDlgItem(hwnd,IDC_BREAKPOINT_ADDRESS),str,256); if (cpu->initExpression(str,exp) == false) { - sprintf(errorMessage,"Invalid expression \"%s\".",str); + snprintf(errorMessage, sizeof(errorMessage), "Invalid expression \"%s\".",str); MessageBoxA(hwnd,errorMessage,"Error",MB_OK); return false; } if (cpu->parseExpression(exp,address) == false) { - sprintf(errorMessage,"Invalid expression \"%s\".",str); + snprintf(errorMessage, sizeof(errorMessage), "Invalid expression \"%s\".",str); MessageBoxA(hwnd,errorMessage,"Error",MB_OK); return false; } @@ -141,14 +145,14 @@ bool BreakpointWindow::fetchDialogData(HWND hwnd) GetWindowTextA(GetDlgItem(hwnd,IDC_BREAKPOINT_SIZE),str,256); if (cpu->initExpression(str,exp) == false) { - sprintf(errorMessage,"Invalid expression \"%s\".",str); + snprintf(errorMessage, sizeof(errorMessage), "Invalid expression \"%s\".",str); MessageBoxA(hwnd,errorMessage,"Error",MB_OK); return false; } if (cpu->parseExpression(exp,size) == false) { - sprintf(errorMessage,"Invalid expression \"%s\".",str); + snprintf(errorMessage, sizeof(errorMessage), "Invalid expression \"%s\".",str); MessageBoxA(hwnd,errorMessage,"Error",MB_OK); return false; } @@ -161,7 +165,7 @@ bool BreakpointWindow::fetchDialogData(HWND hwnd) { if (cpu->initExpression(condition,compiledCondition) == false) { - sprintf(errorMessage,"Invalid expression \"%s\".",str); + snprintf(errorMessage, sizeof(errorMessage), "Invalid expression \"%s\".",str); MessageBoxA(hwnd,errorMessage,"Error",MB_OK); return false; } diff --git a/Windows/Debugger/CtrlDisAsmView.cpp b/Windows/Debugger/CtrlDisAsmView.cpp index 04629fbd1b..ec90a340e4 100644 --- a/Windows/Debugger/CtrlDisAsmView.cpp +++ b/Windows/Debugger/CtrlDisAsmView.cpp @@ -1106,15 +1106,15 @@ void CtrlDisAsmView::updateStatusBarText() { if (!Memory::IsValidAddress(line.info.dataAddress)) { - sprintf(text,"Invalid address %08X",line.info.dataAddress); + snprintf(text, sizeof(text), "Invalid address %08X",line.info.dataAddress); } else { switch (line.info.dataSize) { case 1: - sprintf(text,"[%08X] = %02X",line.info.dataAddress,Memory::Read_U8(line.info.dataAddress)); + snprintf(text, sizeof(text), "[%08X] = %02X",line.info.dataAddress,Memory::Read_U8(line.info.dataAddress)); break; case 2: - sprintf(text,"[%08X] = %04X",line.info.dataAddress,Memory::Read_U16(line.info.dataAddress)); + snprintf(text, sizeof(text), "[%08X] = %04X",line.info.dataAddress,Memory::Read_U16(line.info.dataAddress)); break; case 4: // TODO: Could also be a float... @@ -1123,9 +1123,9 @@ void CtrlDisAsmView::updateStatusBarText() const std::string addressSymbol = symbolMap.GetLabelString(data); if (!addressSymbol.empty()) { - sprintf(text,"[%08X] = %s (%08X)",line.info.dataAddress,addressSymbol.c_str(),data); + snprintf(text, sizeof(text), "[%08X] = %s (%08X)",line.info.dataAddress,addressSymbol.c_str(),data); } else { - sprintf(text,"[%08X] = %08X",line.info.dataAddress,data); + snprintf(text, sizeof(text), "[%08X] = %08X",line.info.dataAddress,data); } break; } @@ -1141,13 +1141,12 @@ void CtrlDisAsmView::updateStatusBarText() const std::string addressSymbol = symbolMap.GetLabelString(line.info.branchTarget); if (addressSymbol.empty()) { - sprintf(text,"%08X",line.info.branchTarget); + snprintf(text, sizeof(text), "%08X", line.info.branchTarget); } else { - sprintf(text,"%08X = %s",line.info.branchTarget,addressSymbol.c_str()); + snprintf(text, sizeof(text), "%08X = %s",line.info.branchTarget,addressSymbol.c_str()); } } - } else if (line.type == DISTYPE_DATA) - { + } else if (line.type == DISTYPE_DATA) { u32 start = symbolMap.GetDataStart(curAddress); if (start == -1) start = curAddress; @@ -1155,25 +1154,23 @@ void CtrlDisAsmView::updateStatusBarText() u32 diff = curAddress-start; const std::string label = symbolMap.GetLabelString(start); - if (!label.empty()) - { + if (!label.empty()) { if (diff != 0) - sprintf(text,"%08X (%s) + %08X",start,label.c_str(),diff); + snprintf(text, sizeof(text), "%08X (%s) + %08X",start,label.c_str(),diff); else - sprintf(text,"%08X (%s)",start,label.c_str()); + snprintf(text, sizeof(text), "%08X (%s)",start,label.c_str()); } else { if (diff != 0) - sprintf(text,"%08X + %08X",start,diff); + snprintf(text, sizeof(text), "%08X + %08X",start,diff); else - sprintf(text,"%08X",start); + snprintf(text, sizeof(text), "%08X",start); } } SendMessage(GetParent(wnd),WM_DEB_SETSTATUSBARTEXT,0,(LPARAM)text); const std::string label = symbolMap.GetLabelString(line.info.opcodeAddress); - if (!label.empty()) - { + if (!label.empty()) { SendMessage(GetParent(wnd),WM_DEB_SETSTATUSBARTEXT,1,(LPARAM)label.c_str()); } } @@ -1373,12 +1370,12 @@ void CtrlDisAsmView::disassembleToFile() MessageBox(wnd,L"Finished!",L"Done",MB_OK); } -void CtrlDisAsmView::getOpcodeText(u32 address, char* dest) +void CtrlDisAsmView::getOpcodeText(u32 address, char* dest, int bufsize) { DisassemblyLineInfo line; address = manager.getStartAddress(address); manager.getLine(address,displaySymbols,line); - sprintf(dest,"%s %s",line.name.c_str(),line.params.c_str()); + snprintf(dest, bufsize, "%s %s",line.name.c_str(),line.params.c_str()); } void CtrlDisAsmView::scrollStepping(u32 newPc) diff --git a/Windows/Debugger/CtrlDisAsmView.h b/Windows/Debugger/CtrlDisAsmView.h index c76584cc56..7b06b3a517 100644 --- a/Windows/Debugger/CtrlDisAsmView.h +++ b/Windows/Debugger/CtrlDisAsmView.h @@ -103,7 +103,7 @@ public: void scanFunctions(); void clearFunctions() { manager.clear(); }; - void getOpcodeText(u32 address, char* dest); + void getOpcodeText(u32 address, char* dest, int bufsize); int getRowHeight() { return rowHeight; }; u32 yToAddress(int y); diff --git a/Windows/Debugger/Debugger_Disasm.cpp b/Windows/Debugger/Debugger_Disasm.cpp index aea3fc0e37..0aa1f762e2 100644 --- a/Windows/Debugger/Debugger_Disasm.cpp +++ b/Windows/Debugger/Debugger_Disasm.cpp @@ -721,11 +721,10 @@ BOOL CDisasm::DlgProc(UINT message, WPARAM wParam, LPARAM lParam) void CDisasm::updateThreadLabel(bool clear) { char label[512]; - if (clear) - { - sprintf(label,"Thread: -"); + if (clear) { + snprintf(label, sizeof(label), "Thread: -"); } else { - sprintf(label,"Thread: %s",threadList->getCurrentThreadName()); + snprintf(label, sizeof(label), "Thread: %s", threadList->getCurrentThreadName()); } SetDlgItemText(m_hDlg, IDC_THREADNAME, ConvertUTF8ToWString(label).c_str()); diff --git a/Windows/Debugger/Debugger_Lists.cpp b/Windows/Debugger/Debugger_Lists.cpp index 012a279456..c183642a86 100644 --- a/Windows/Debugger/Debugger_Lists.cpp +++ b/Windows/Debugger/Debugger_Lists.cpp @@ -524,7 +524,7 @@ void CtrlBreakpointList::GetColumnText(wchar_t* dest, int row, int col) wcscpy(dest,L"-"); } else { char temp[256]; - disasm->getOpcodeText(displayedBreakPoints_[index].addr, temp); + disasm->getOpcodeText(displayedBreakPoints_[index].addr, temp, sizeof(temp)); std::wstring s = ConvertUTF8ToWString(temp); wcscpy(dest,s.c_str()); } @@ -694,7 +694,7 @@ void CtrlStackTraceView::GetColumnText(wchar_t* dest, int row, int col) case SF_CUROPCODE: { char temp[512]; - disasm->getOpcodeText(frames[row].pc,temp); + disasm->getOpcodeText(frames[row].pc, temp, sizeof(temp)); wcscpy(dest, ConvertUTF8ToWString(temp).c_str()); } break; diff --git a/Windows/Debugger/DumpMemoryWindow.cpp b/Windows/Debugger/DumpMemoryWindow.cpp index 58f6adbca1..7fcc420157 100644 --- a/Windows/Debugger/DumpMemoryWindow.cpp +++ b/Windows/Debugger/DumpMemoryWindow.cpp @@ -74,10 +74,9 @@ INT_PTR CALLBACK DumpMemoryWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam, break; FILE* output = fopen(bp->fileName,"wb"); - if (output == NULL) - { - char errorMessage[256]; - sprintf(errorMessage,"Could not open file \"%s\".",bp->fileName); + if (output == NULL) { + char errorMessage[2048]; + snprintf(errorMessage, sizeof(errorMessage), "Could not open file \"%s\".",bp->fileName); MessageBoxA(hwnd,errorMessage,"Error",MB_OK); break; } diff --git a/ext/disarm.cpp b/ext/disarm.cpp index 5bef319c71..b76ca40405 100644 --- a/ext/disarm.cpp +++ b/ext/disarm.cpp @@ -1269,7 +1269,7 @@ const char *ArmRegName(int r) { return reg_names[r]; } -void ArmDis(unsigned int addr, unsigned int w, char *output, bool includeWord) { +void ArmDis(unsigned int addr, unsigned int w, char *output, int bufsize, bool includeWord) { pInstruction instr = instr_disassemble(w, addr, &options); char temp[256]; if (includeWord) { diff --git a/ext/disarm.h b/ext/disarm.h index 609b5ef542..9bae68f215 100644 --- a/ext/disarm.h +++ b/ext/disarm.h @@ -31,4 +31,4 @@ // program with this function. const char *ArmRegName(int r); -void ArmDis(unsigned int addr, unsigned int w, char *output, bool includeWord = true); +void ArmDis(unsigned int addr, unsigned int w, char *output, int bufsize, bool includeWord); diff --git a/native b/native index 33d496d76f..6ee862b564 160000 --- a/native +++ b/native @@ -1 +1 @@ -Subproject commit 33d496d76f529a6b921c4c8fef70e6c5fd2caef7 +Subproject commit 6ee862b564bca639659a231937e02dfec0774475 diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 767a11d869..5ccd8e378b 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -246,7 +246,7 @@ bool CheckLast(ArmGen::ARMXEmitter &emit, const char *comp) { u32 instr; memcpy(&instr, emit.GetCodePtr() - 4, 4); char disasm[512]; - ArmDis(0, instr, disasm); + ArmDis(0, instr, disasm, sizeof(disasm), true); EXPECT_EQ_STR(std::string(disasm), std::string(comp)); return true; }