Code review (#3114)

* Fix always-true conditions in sceNp module

* gl_render_targets: useless check on unsigned variable, possible bug

* fixed UB in crypto utility functions

* copy-paste error in vk::init_default_resources

* pass strings by const ref

* Dont copy vectors. Make sure copies are not needed because functions are used in a multi-threaded context.
This commit is contained in:
mp-t 2017-08-01 19:22:33 +02:00 committed by Ivan
parent 77f58326af
commit 607d2486ea
16 changed files with 32 additions and 32 deletions

View File

@ -257,8 +257,8 @@ struct fmt::cfmt_src
void skip(std::size_t extra)
{
++sup += extra;
++args += extra;
sup += extra + 1;
args += extra + 1;
}
std::size_t fmt_string(std::string& out, std::size_t extra) const

View File

@ -150,8 +150,8 @@ s64 decrypt_block(const fs::file* in, u8* out, EDAT_HEADER *edat, NPD_HEADER *np
const int metadata_section_size = ((edat->flags & EDAT_COMPRESSED_FLAG) != 0 || (edat->flags & EDAT_FLAG_0x20) != 0) ? 0x20 : 0x10;
const int metadata_offset = 0x100;
std::unique_ptr<u8> enc_data;
std::unique_ptr<u8> dec_data;
std::unique_ptr<u8[]> enc_data;
std::unique_ptr<u8[]> dec_data;
u8 hash[0x10] = { 0 };
u8 key_result[0x10] = { 0 };
u8 hash_result[0x14] = { 0 };
@ -315,7 +315,7 @@ int decrypt_data(const fs::file* in, const fs::file* out, EDAT_HEADER *edat, NPD
{
const int total_blocks = (int)((edat->file_size + edat->block_size - 1) / edat->block_size);
u64 size_left = (int)edat->file_size;
std::unique_ptr<u8> data(new u8[edat->block_size]);
std::unique_ptr<u8[]> data(new u8[edat->block_size]);
for (int i = 0; i < total_blocks; i++)
{
@ -427,8 +427,8 @@ int check_data(unsigned char *key, EDAT_HEADER *edat, NPD_HEADER *npd, const fs:
long bytes_read = 0;
long bytes_to_read = metadata_size;
std::unique_ptr<u8> metadata(new u8[metadata_size]);
std::unique_ptr<u8> empty_metadata(new u8[metadata_size]);
std::unique_ptr<u8[]> metadata(new u8[metadata_size]);
std::unique_ptr<u8[]> empty_metadata(new u8[metadata_size]);
while (bytes_to_read > 0)
{
@ -487,7 +487,7 @@ int check_data(unsigned char *key, EDAT_HEADER *edat, NPD_HEADER *npd, const fs:
if ((edat->flags & EDAT_FLAG_0x20) != 0) //Sony failed again, they used buffer from 0x100 with half size of real metadata.
{
int metadata_buf_size = block_num * 0x10;
std::unique_ptr<u8> metadata_buf(new u8[metadata_buf_size]);
std::unique_ptr<u8[]> metadata_buf(new u8[metadata_buf_size]);
f->seek(file_offset + metadata_offset);
f->read(metadata_buf.get(), metadata_buf_size);
sha1(metadata_buf.get(), metadata_buf_size, signature_hash);
@ -516,7 +516,7 @@ int check_data(unsigned char *key, EDAT_HEADER *edat, NPD_HEADER *npd, const fs:
{
// Setup header signature hash.
memset(signature_hash, 0, 20);
std::unique_ptr<u8> header_buf(new u8[0xD8]);
std::unique_ptr<u8[]> header_buf(new u8[0xD8]);
f->seek(file_offset);
f->read(header_buf.get(), 0xD8);
sha1(header_buf.get(), 0xD8, signature_hash );
@ -577,7 +577,7 @@ int validate_npd_hashes(const char* file_name, const u8* klicensee, NPD_HEADER *
int dev_hash_result = 0;
const int file_name_length = (int) strlen(file_name);
std::unique_ptr<u8> buf(new u8[0x30 + file_name_length]);
std::unique_ptr<u8[]> buf(new u8[0x30 + file_name_length]);
// Build the title buffer (content_id + file_name).
memcpy(buf.get(), npd->content_id, 0x30);

View File

@ -118,7 +118,7 @@ void aesecb128_encrypt(unsigned char *key, unsigned char *in, unsigned char *out
bool hmac_hash_compare(unsigned char *key, int key_len, unsigned char *in, int in_len, unsigned char *hash, int hash_len)
{
std::unique_ptr<u8> out(new u8[key_len]);
std::unique_ptr<u8[]> out(new u8[key_len]);
sha1_hmac(key, key_len, in, in_len, out.get());
@ -132,7 +132,7 @@ void hmac_hash_forge(unsigned char *key, int key_len, unsigned char *in, int in_
bool cmac_hash_compare(unsigned char *key, int key_len, unsigned char *in, int in_len, unsigned char *hash, int hash_len)
{
std::unique_ptr<u8> out(new u8[key_len]);
std::unique_ptr<u8[]> out(new u8[key_len]);
aes_context ctx;
aes_setkey_enc(&ctx, key, 128);

View File

@ -161,7 +161,7 @@ bool _L10nCodeParse(s32 code, HostCode& retCode)
#ifdef _MSC_VER
// Use code page to transform std::string to std::wstring.
s32 _OEM2Wide(HostCode oem_code, const std::string src, std::wstring& dst)
s32 _OEM2Wide(HostCode oem_code, const std::string& src, std::wstring& dst)
{
//Such length returned should include the '\0' character.
s32 length = MultiByteToWideChar(oem_code, 0, src.c_str(), -1, NULL, 0);
@ -178,7 +178,7 @@ s32 _OEM2Wide(HostCode oem_code, const std::string src, std::wstring& dst)
}
// Use Code page to transform std::wstring to std::string.
s32 _Wide2OEM(HostCode oem_code, const std::wstring src, std::string& dst)
s32 _Wide2OEM(HostCode oem_code, const std::wstring& src, std::string& dst)
{
//Such length returned should include the '\0' character.
s32 length = WideCharToMultiByte(oem_code, 0, src.c_str(), -1, NULL, 0, NULL, NULL);
@ -195,7 +195,7 @@ s32 _Wide2OEM(HostCode oem_code, const std::wstring src, std::string& dst)
}
// Convert Codepage to Codepage (all char*)
std::string _OemToOem(HostCode src_code, HostCode dst_code, const std::string str)
std::string _OemToOem(HostCode src_code, HostCode dst_code, const std::string& str)
{
std::wstring wide; std::string result;
_OEM2Wide(src_code, str, wide);

View File

@ -972,7 +972,7 @@ s32 sceNpManagerGetOnlineId(vm::ptr<SceNpOnlineId> onlineId)
return SCE_NP_ERROR_OFFLINE;
}
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN || g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN && g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
{
return SCE_NP_ERROR_INVALID_STATE;
}
@ -994,7 +994,7 @@ s32 sceNpManagerGetNpId(ppu_thread& ppu, vm::ptr<SceNpId> npId)
return SCE_NP_ERROR_OFFLINE;
}
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN || g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN && g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
{
return SCE_NP_ERROR_INVALID_STATE;
}
@ -1082,7 +1082,7 @@ s32 sceNpManagerGetAccountRegion(vm::ptr<SceNpCountryCode> countryCode, vm::ptr<
return SCE_NP_ERROR_OFFLINE;
}
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN || g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN && g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
{
return SCE_NP_ERROR_INVALID_STATE;
}
@ -1104,7 +1104,7 @@ s32 sceNpManagerGetAccountAge(vm::ptr<s32> age)
return SCE_NP_ERROR_OFFLINE;
}
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN || g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN && g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
{
return SCE_NP_ERROR_INVALID_STATE;
}
@ -1126,7 +1126,7 @@ s32 sceNpManagerGetContentRatingFlag(vm::ptr<s32> isRestricted, vm::ptr<s32> age
return SCE_NP_ERROR_OFFLINE;
}
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN || g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN && g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
{
return SCE_NP_ERROR_INVALID_STATE;
}
@ -1152,7 +1152,7 @@ s32 sceNpManagerGetChatRestrictionFlag(vm::ptr<s32> isRestricted)
return SCE_NP_ERROR_OFFLINE;
}
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN || g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
if (g_psn_connection_status != SCE_NP_MANAGER_STATUS_LOGGING_IN && g_psn_connection_status != SCE_NP_MANAGER_STATUS_ONLINE)
{
return SCE_NP_ERROR_INVALID_STATE;
}

View File

@ -31,7 +31,7 @@ struct ps3_fmt_src
void skip(std::size_t extra)
{
++g_count += (u32)extra;
g_count += (u32)extra + 1;
}
std::size_t fmt_string(std::string& out, std::size_t extra) const

View File

@ -166,7 +166,7 @@ void spu_recompiler::compile(spu_function_t& f)
dis_asm.dump_pc = m_pos;
dis_asm.disasm(m_pos);
compiler.comment(dis_asm.last_opcode.c_str());
log += dis_asm.last_opcode.c_str();
log += dis_asm.last_opcode;
log += '\n';
}

View File

@ -373,7 +373,7 @@ void VertexProgramDecompiler::SetDSTSca(const std::string& code)
SetDST(true, code);
}
std::string VertexProgramDecompiler::NotZeroPositive(const std::string code)
std::string VertexProgramDecompiler::NotZeroPositive(const std::string& code)
{
return "max(" + code + ", 1.E-10)";
}

View File

@ -61,7 +61,7 @@ struct VertexProgramDecompiler
const std::vector<u32>& m_data;
ParamArray m_parr;
std::string NotZeroPositive(const std::string code);
std::string NotZeroPositive(const std::string& code);
std::string GetMask(bool is_sca);
std::string GetVecMask();
std::string GetScaMask();

View File

@ -42,7 +42,7 @@ namespace
fmt::throw_exception("Wrong vector size %d" HERE, size);
}
u32 get_vertex_count(const std::vector<std::pair<u32, u32> > first_count_commands)
u32 get_vertex_count(const std::vector<std::pair<u32, u32> >& first_count_commands)
{
u32 vertex_count = 0;
for (const auto &pair : first_count_commands)

View File

@ -340,7 +340,7 @@ private:
return false;
u32 offset = texaddr - surface_address;
if (offset >= 0)
if (texaddr >= surface_address)
{
std::tie(is_subslice, x_offset, y_offset) = surface->get_texture_subresource(offset, scale_to_fit);
if (is_subslice)

View File

@ -164,7 +164,7 @@ namespace
return std::make_tuple(element_count, mapping.second);
}
std::tuple<u32, u32, u32> upload_index_buffer(gsl::span<const gsl::byte> raw_index_buffer, void *ptr, rsx::index_array_type type, rsx::primitive_type draw_mode, const std::vector<std::pair<u32, u32>> first_count_commands, u32 initial_vertex_count)
std::tuple<u32, u32, u32> upload_index_buffer(gsl::span<const gsl::byte> raw_index_buffer, void *ptr, rsx::index_array_type type, rsx::primitive_type draw_mode, const std::vector<std::pair<u32, u32>>& first_count_commands, u32 initial_vertex_count)
{
u32 min_index, max_index, vertex_draw_count = initial_vertex_count;

View File

@ -216,7 +216,7 @@ struct rsx_vertex_input
bool int_type;
u32 flags; //Initially zero, to be optionally filled by the backend
bool operator==(const rsx_vertex_input other) const
bool operator==(const rsx_vertex_input& other) const
{
return location == other.location && size == other.size && frequency == other.frequency && is_modulo == other.is_modulo &&
is_array == other.is_array && int_type == other.int_type && flags == other.flags;

View File

@ -163,7 +163,7 @@ namespace vk
rsc.maxFragmentUniformVectors = 16;
rsc.maxVertexOutputVectors = 16;
rsc.maxFragmentInputVectors = 15;
rsc.maxProgramTexelOffset = -8;
rsc.minProgramTexelOffset = -8;
rsc.maxProgramTexelOffset = 7;
rsc.maxClipDistances = 8;
rsc.maxComputeWorkGroupCountX = 65535;

View File

@ -1463,6 +1463,6 @@ namespace vk
* dst_image must be in TRANSFER_DST_OPTIMAL layout and upload_buffer have TRANSFER_SRC_BIT usage flag.
*/
void copy_mipmaped_image_using_buffer(VkCommandBuffer cmd, VkImage dst_image,
const std::vector<rsx_subresource_layout> subresource_layout, int format, bool is_swizzled, u16 mipmap_count,
const std::vector<rsx_subresource_layout>& subresource_layout, int format, bool is_swizzled, u16 mipmap_count,
vk::vk_data_heap &upload_heap, vk::buffer* upload_buffer);
}

View File

@ -136,7 +136,7 @@ namespace vk
}
void copy_mipmaped_image_using_buffer(VkCommandBuffer cmd, VkImage dst_image,
const std::vector<rsx_subresource_layout> subresource_layout, int format, bool is_swizzled, u16 mipmap_count,
const std::vector<rsx_subresource_layout>& subresource_layout, int format, bool is_swizzled, u16 mipmap_count,
vk::vk_data_heap &upload_heap, vk::buffer* upload_buffer)
{
u32 mipmap_level = 0;