Skip to content

Commit

Permalink
Misc: Fix a bunch of code analysis warnings
Browse files Browse the repository at this point in the history
Some of which were even actual errors.
  • Loading branch information
stenzek committed Aug 2, 2024
1 parent 4eb3b2a commit 3a83c42
Show file tree
Hide file tree
Showing 30 changed files with 93 additions and 78 deletions.
4 changes: 2 additions & 2 deletions src/common/binary_reader_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class BinarySpanReader
ALWAYS_INLINE BinarySpanReader& operator>>(s64& val) { val = ReadT<s64>(); return *this; }
ALWAYS_INLINE BinarySpanReader& operator>>(u64& val) { val = ReadT<u64>(); return *this; }
ALWAYS_INLINE BinarySpanReader& operator>>(float& val) { val = ReadT<float>(); return *this; }
ALWAYS_INLINE BinarySpanReader& operator>>(std::string_view val) { val = ReadCString(); return *this; }
ALWAYS_INLINE BinarySpanReader& operator>>(std::string_view& val) { val = ReadCString(); return *this; }
// clang-format on

template<typename T>
Expand Down Expand Up @@ -272,7 +272,7 @@ class BinaryFileReader
ALWAYS_INLINE BinaryFileReader& operator>>(s64& val) { val = ReadT<s64>(); return *this; }
ALWAYS_INLINE BinaryFileReader& operator>>(u64& val) { val = ReadT<u64>(); return *this; }
ALWAYS_INLINE BinaryFileReader& operator>>(float& val) { val = ReadT<float>(); return *this; }
ALWAYS_INLINE BinaryFileReader& operator>>(std::string_view val) { val = ReadCString(); return *this; }
ALWAYS_INLINE BinaryFileReader& operator>>(std::string_view& val) { val = ReadCString(); return *this; }
// clang-format on

template<typename T>
Expand Down
6 changes: 3 additions & 3 deletions src/common/file_system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,20 +505,20 @@ std::string Path::Canonicalize(std::string_view path)
{
// current directory, so it can be skipped, unless it's the only component
if (components.size() == 1)
new_components.push_back(std::move(component));
new_components.push_back(component);
}
else if (component == "..")
{
// parent directory, pop one off if we're not at the beginning, otherwise preserve.
if (!new_components.empty())
new_components.pop_back();
else
new_components.push_back(std::move(component));
new_components.push_back(component);
}
else
{
// anything else, preserve
new_components.push_back(std::move(component));
new_components.push_back(component);
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/common/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,11 @@ ALWAYS_INLINE_RELEASE void Log::FormatLogMessageAndPrintW(const char* channelNam

wmessage_buflen =
MultiByteToWideChar(CP_UTF8, 0, buffer.data(), static_cast<int>(buffer.size()), wmessage_buf, wmessage_buflen);
if (wmessage_buflen <= 0)
return;

wmessage_buf[wmessage_buflen] = '\0';
callback(std::wstring_view(wmessage_buf, wmessage_buflen));
if (wmessage_buflen > 0) [[likely]]
{
wmessage_buf[wmessage_buflen] = '\0';
callback(std::wstring_view(wmessage_buf, wmessage_buflen));
}

if (wmessage_buf != wbuf)
std::free(wmessage_buf);
Expand Down
6 changes: 3 additions & 3 deletions src/common/memmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ std::string MemMap::GetFileMappingName(const char* prefix)
void* MemMap::CreateSharedMemory(const char* name, size_t size, Error* error)
{
const HANDLE mapping =
static_cast<void*>(CreateFileMappingW(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, static_cast<DWORD>(size >> 32),
static_cast<DWORD>(size), StringUtil::UTF8StringToWideString(name).c_str()));
CreateFileMappingW(INVALID_HANDLE_VALUE, nullptr, PAGE_READWRITE, static_cast<DWORD>(size >> 32),
static_cast<DWORD>(size), StringUtil::UTF8StringToWideString(name).c_str());
if (!mapping)
Error::SetWin32(error, "CreateFileMappingW() failed: ", GetLastError());

return mapping;
return static_cast<void*>(mapping);
}

void MemMap::DestroySharedMemory(void* ptr)
Expand Down
8 changes: 6 additions & 2 deletions src/common/small_string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,12 @@ void SmallStringBase::append_vsprintf(const char* format, va_list ap)
va_copy(ap_copy, ap);
const int ret = std::vsnprintf(buffer, buffer_size, format, ap_copy);
va_end(ap_copy);
if (ret < 0 || ((u32)ret >= (buffer_size - 1)))
if (ret < 0 || (static_cast<u32>(ret) >= (buffer_size - 1)))
{
buffer_size *= 2;
buffer = heap_buffer = reinterpret_cast<char*>(std::realloc(heap_buffer, buffer_size));
if (!buffer) [[unlikely]]
Panic("Memory allocation failed.");
continue;
}

Expand Down Expand Up @@ -303,7 +305,7 @@ void SmallStringBase::prepend_vsprintf(const char* format, va_list ArgPtr)
// We have a 1KB byte buffer on the stack here. If this is too little, we'll grow it via the heap,
// but 1KB should be enough for most strings.
char stack_buffer[1024];
char* heap_buffer = NULL;
char* heap_buffer = nullptr;
char* buffer = stack_buffer;
u32 buffer_size = static_cast<u32>(std::size(stack_buffer));
u32 written;
Expand All @@ -315,6 +317,8 @@ void SmallStringBase::prepend_vsprintf(const char* format, va_list ArgPtr)
{
buffer_size *= 2;
buffer = heap_buffer = reinterpret_cast<char*>(std::realloc(heap_buffer, buffer_size));
if (!buffer) [[unlikely]]
Panic("Memory allocation failed.");
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/bios.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ std::optional<BIOS::Image> BIOS::FindBIOSImageInDirectory(ConsoleRegion region,

std::string full_path(Path::Combine(directory, fd.FileName));
std::optional<Image> found_image = LoadImageFromFile(full_path.c_str(), nullptr);
if (!found_image)
if (!found_image.has_value())
continue;

if (found_image->info && IsValidBIOSForRegion(region, found_image->info->region))
Expand Down
11 changes: 10 additions & 1 deletion src/core/bus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1399,8 +1399,11 @@ template<MemoryAccessSize size>
u32 Bus::HWHandlers::MemCtrlRead(PhysicalMemoryAddress address)
{
const u32 offset = address & MEMCTRL_MASK;
const u32 index = FIXUP_WORD_OFFSET(size, offset) / 4;
if (index >= std::size(s_MEMCTRL.regs)) [[unlikely]]
return 0;

u32 value = s_MEMCTRL.regs[FIXUP_WORD_OFFSET(size, offset) / 4];
u32 value = s_MEMCTRL.regs[index];
value = FIXUP_WORD_READ_VALUE(size, offset, value);
BUS_CYCLES(2);
return value;
Expand All @@ -1411,6 +1414,9 @@ void Bus::HWHandlers::MemCtrlWrite(PhysicalMemoryAddress address, u32 value)
{
const u32 offset = address & MEMCTRL_MASK;
const u32 index = FIXUP_WORD_OFFSET(size, offset) / 4;
if (index >= std::size(s_MEMCTRL.regs)) [[unlikely]]
return;

value = FIXUP_WORD_WRITE_VALUE(size, offset, value);

const u32 write_mask = (index == 8) ? COMDELAY::WRITE_MASK : MEMDELAY::WRITE_MASK;
Expand Down Expand Up @@ -1516,17 +1522,20 @@ u32 Bus::HWHandlers::CDROMRead(PhysicalMemoryAddress address)
const u32 b3 = ZeroExtend32(CDROM::ReadRegister(offset + 3u));
value = b0 | (b1 << 8) | (b2 << 16) | (b3 << 24);
}
break;

case MemoryAccessSize::HalfWord:
{
const u32 lsb = ZeroExtend32(CDROM::ReadRegister(offset));
const u32 msb = ZeroExtend32(CDROM::ReadRegister(offset + 1u));
value = lsb | (msb << 8);
}
break;

case MemoryAccessSize::Byte:
default:
value = ZeroExtend32(CDROM::ReadRegister(offset));
break;
}

BUS_CYCLES(Bus::g_cdrom_access_time[static_cast<u32>(size)]);
Expand Down
8 changes: 5 additions & 3 deletions src/core/cheats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,10 @@ bool CheatList::LoadFromString(const std::string& str, Format format)
return LoadFromPCSXRString(str);
else if (format == Format::Libretro)
return LoadFromLibretroString(str);
format = Format::EPSXe;
return LoadFromEPSXeString(str);
else if (format == Format::EPSXe)
return LoadFromEPSXeString(str);
else
return false;
}

bool CheatList::SaveToPCSXRFile(const char* filename)
Expand Down Expand Up @@ -2782,7 +2784,7 @@ const char* CheatCode::GetTypeDisplayName(Type type)

std::optional<CheatCode::Type> CheatCode::ParseTypeName(const char* str)
{
for (u32 i = 0; i < static_cast<u32>(s_cheat_code_type_names.size()); i++)
for (size_t i = 0; i < s_cheat_code_type_names.size(); i++)
{
if (std::strcmp(s_cheat_code_type_names[i], str) == 0)
return static_cast<Type>(i);
Expand Down
6 changes: 3 additions & 3 deletions src/core/fullscreen_ui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ void FullscreenUI::DoChangeDisc()
if (const GameDatabase::Entry* entry = System::GetGameDatabaseEntry(); entry && !entry->disc_set_serials.empty())
{
const auto lock = GameList::GetLock();
const auto matches = GameList::GetMatchingEntriesForSerial(entry->disc_set_serials);
auto matches = GameList::GetMatchingEntriesForSerial(entry->disc_set_serials);
if (matches.size() > 1)
{
options.reserve(matches.size() + 1);
Expand Down Expand Up @@ -5759,7 +5759,7 @@ void FullscreenUI::DrawSaveStateSelector(bool is_loading)
if (i == 0)
ResetFocusHere();

const SaveStateListEntry& entry = s_save_state_selector_slots[i];
SaveStateListEntry& entry = s_save_state_selector_slots[i];
if (static_cast<s32>(i) == s_save_state_selector_submenu_index)
{
// can't use a choice dialog here, because we're already in a modal...
Expand Down Expand Up @@ -6021,7 +6021,7 @@ void FullscreenUI::DrawResumeStateSelector()
if (ImGui::BeginPopupModal(FSUI_CSTR("Load Resume State"), &is_open,
ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoResize))
{
const SaveStateListEntry& entry = s_save_state_selector_slots.front();
SaveStateListEntry& entry = s_save_state_selector_slots.front();
SmallString time;
TimeToPrintableString(&time, entry.timestamp);
ImGui::TextWrapped(
Expand Down
6 changes: 3 additions & 3 deletions src/core/game_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1736,10 +1736,10 @@ std::string GameList::GetGameIconPath(std::string_view serial, std::string_view

// Try extracting an icon.
Error error;
MemoryCardImage::DataArray data;
if (MemoryCardImage::LoadFromFile(&data, memcard_path.c_str(), &error))
std::unique_ptr<MemoryCardImage::DataArray> data = std::make_unique<MemoryCardImage::DataArray>();
if (MemoryCardImage::LoadFromFile(data.get(), memcard_path.c_str(), &error))
{
std::vector<MemoryCardImage::FileInfo> files = MemoryCardImage::EnumerateFiles(data, false);
std::vector<MemoryCardImage::FileInfo> files = MemoryCardImage::EnumerateFiles(*data.get(), false);
if (!files.empty())
{
const MemoryCardImage::FileInfo& fi = files.front();
Expand Down
7 changes: 2 additions & 5 deletions src/core/gpu_hw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2253,7 +2253,6 @@ void GPU_HW::DrawLine(const GSVector4 bounds, u32 col0, u32 col1, float depth)
const float abs_dx = std::fabs(dx);
const float abs_dy = std::fabs(dy);
float fill_dx, fill_dy;
float dxdk, dydk;
float pad_x0 = 0.0f;
float pad_x1 = 0.0f;
float pad_y0 = 0.0f;
Expand All @@ -2268,8 +2267,7 @@ void GPU_HW::DrawLine(const GSVector4 bounds, u32 col0, u32 col1, float depth)
{
fill_dx = 0.0f;
fill_dy = 1.0f;
dxdk = 1.0f;
dydk = dy / abs_dx;
const float dydk = dy / abs_dx;

if (dx > 0.0f)
{
Expand All @@ -2288,8 +2286,7 @@ void GPU_HW::DrawLine(const GSVector4 bounds, u32 col0, u32 col1, float depth)
{
fill_dx = 1.0f;
fill_dy = 0.0f;
dydk = 1.0f;
dxdk = dx / abs_dy;
const float dxdk = dx / abs_dy;

if (dy > 0.0f)
{
Expand Down
10 changes: 2 additions & 8 deletions src/core/imgui_overlays.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,6 @@ void ImGuiManager::DrawPerformanceOverlay()
ImDrawList* dl = ImGui::GetBackgroundDrawList();
SmallString text;
ImVec2 text_size;
bool first = true;

#define DRAW_LINE(font, text, color) \
do \
Expand All @@ -260,21 +259,16 @@ void ImGuiManager::DrawPerformanceOverlay()
{
const float speed = System::GetEmulationSpeed();
if (g_settings.display_show_fps)
{
text.append_format("G: {:.2f} | V: {:.2f}", System::GetFPS(), System::GetVPS());
first = false;
}
if (g_settings.display_show_speed)
{
text.append_format("{}{}%", first ? "" : " | ", static_cast<u32>(std::round(speed)));
text.append_format("{}{}%", text.empty() ? "" : " | ", static_cast<u32>(std::round(speed)));

const float target_speed = System::GetTargetSpeed();
if (target_speed <= 0.0f)
text.append(" (Max)");
else
text.append_format(" ({:.0f}%)", target_speed * 100.0f);

first = false;
}
if (!text.empty())
{
Expand Down Expand Up @@ -325,7 +319,7 @@ void ImGuiManager::DrawPerformanceOverlay()
g_settings.cpu_execution_mode != CPUExecutionMode::Recompiler || g_settings.cpu_recompiler_icache ||
g_settings.cpu_recompiler_memory_exceptions)
{
first = true;
bool first = true;
text.assign("CPU[");
if (g_settings.cpu_overclock_active)
{
Expand Down
43 changes: 24 additions & 19 deletions src/core/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -962,11 +962,11 @@ bool System::ReadExecutableFromImage(CDImage* cdi, std::string* out_executable_n
bool System::ReadExecutableFromImage(IsoReader& iso, std::string* out_executable_name,
std::vector<u8>* out_executable_data)
{
const std::string executable_path = GetExecutableNameForImage(iso, false);
std::string executable_path = GetExecutableNameForImage(iso, false);
DEV_LOG("Executable path: '{}'", executable_path);
if (!executable_path.empty() && out_executable_data)
{
if (!iso.ReadFile(executable_path.c_str(), out_executable_data))
if (!iso.ReadFile(executable_path, out_executable_data))
{
ERROR_LOG("Failed to read executable '{}' from disc", executable_path);
return false;
Expand All @@ -993,26 +993,31 @@ System::GameHash System::GetGameHashFromBuffer(std::string_view exe_name, std::s
return hash;
}

DiscRegion System::GetRegionForSerial(std::string_view serial)
DiscRegion System::GetRegionForSerial(const std::string_view serial)
{
std::string prefix;
for (size_t pos = 0; pos < serial.length(); pos++)
{
const int ch = std::tolower(serial[pos]);
if (ch < 'a' || ch > 'z')
break;
static constexpr const std::pair<const char*, DiscRegion> region_prefixes[] = {
{"sces", DiscRegion::PAL},
{"sced", DiscRegion::PAL},
{"sles", DiscRegion::PAL},
{"sled", DiscRegion::PAL},

{"scps", DiscRegion::NTSC_J},
{"slps", DiscRegion::NTSC_J},
{"slpm", DiscRegion::NTSC_J},
{"sczs", DiscRegion::NTSC_J},
{"papx", DiscRegion::NTSC_J},

prefix.push_back(static_cast<char>(ch));
{"scus", DiscRegion::NTSC_U},
{"slus", DiscRegion::NTSC_U},
};

for (const auto& [prefix, region] : region_prefixes)
{
if (StringUtil::StartsWithNoCase(serial, prefix))
return region;
}

if (prefix == "sces" || prefix == "sced" || prefix == "sles" || prefix == "sled")
return DiscRegion::PAL;
else if (prefix == "scps" || prefix == "slps" || prefix == "slpm" || prefix == "sczs" || prefix == "papx")
return DiscRegion::NTSC_J;
else if (prefix == "scus" || prefix == "slus")
return DiscRegion::NTSC_U;
else
return DiscRegion::Other;
return DiscRegion::Other;
}

DiscRegion System::GetRegionFromSystemArea(CDImage* cdi)
Expand Down Expand Up @@ -1311,7 +1316,7 @@ bool System::UpdateGameSettingsLayer()
std::unique_ptr<INISettingsInterface> input_interface;
if (!input_profile_name.empty())
{
const std::string filename(GetInputProfilePath(input_profile_name));
std::string filename = GetInputProfilePath(input_profile_name);
if (FileSystem::FileExists(filename.c_str()))
{
INFO_LOG("Loading input profile from '{}'...", Path::GetFileName(filename));
Expand Down
2 changes: 1 addition & 1 deletion src/core/system.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ bool ReadExecutableFromImage(CDImage* cdi, std::string* out_executable_name, std
std::string GetGameHashId(GameHash hash);
bool GetGameDetailsFromImage(CDImage* cdi, std::string* out_id, GameHash* out_hash);
GameHash GetGameHashFromFile(const char* path);
DiscRegion GetRegionForSerial(std::string_view serial);
DiscRegion GetRegionForSerial(const std::string_view serial);
DiscRegion GetRegionFromSystemArea(CDImage* cdi);
DiscRegion GetRegionForImage(CDImage* cdi);
DiscRegion GetRegionForExe(const char* path);
Expand Down
2 changes: 2 additions & 0 deletions src/core/texture_replacements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ void TextureReplacements::Reload()
void TextureReplacements::PurgeUnreferencedTexturesFromCache()
{
TextureCache old_map = std::move(s_texture_cache);
s_texture_cache = {};

for (const auto& it : s_vram_write_replacements)
{
auto it2 = old_map.find(it.second);
Expand Down
1 change: 1 addition & 0 deletions src/util/audio_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ AudioStream::AudioStream(u32 sample_rate, const AudioStreamParameters& parameter

AudioStream::~AudioStream()
{
StretchDestroy();
DestroyBuffer();
}

Expand Down
Loading

0 comments on commit 3a83c42

Please sign in to comment.