ronantakizawa/codereview-bench huggingface dataset:
Repository: skyline-emu/skyline
File: app/src/main/cpp/skyline/services/account/IProfile.cpp (C++)

Code before:
            .uid = userId,
        };

        size_t usernameSize{std::min(accountProfileBase.nickname.size() - 1, (*state.settings->usernameValue).size())};
        std::memcpy(accountProfileBase.nickname.data(), (*state.settings->usernameValue).c_str(), usernameSize);

        response.Push(accountProfileBase);

        return {};
    }

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        std::vector<char> profileImage(profileImageIcon->size);

        profileImageIcon->Read(skyline::span<char>(profileImage), 0);

        request.outputBuf.at(0).copy_from(profileImage);
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture(){
        const std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            return state.os->assetFileSystem->OpenFile("skyline-logo.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}


Review comment (suggestion):
```suggestion
        response.Push(static_cast<u32>(profileImageIcon->size));
```

Code after:
        };

        size_t usernameSize{std::min(accountProfileBase.nickname.size() - 1, (*state.settings->usernameValue).size())};
        std::memcpy(accountProfileBase.nickname.data(), (*state.settings->usernameValue).c_str(), usernameSize);

        response.Push(accountProfileBase);

        return {};
    }

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};

        profileImageIcon->Read(span(request.outputBuf.at(0)).first(profileImageIcon->size), 0);
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture() {
        std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            // If we can't find the profile picture then just return the placeholder profile picture
            return state.os->assetFileSystem->OpenFile("profile_picture.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}
Repository: skyline-emu/skyline
File: app/src/main/cpp/skyline/services/account/IProfile.cpp (C++)

Code before:
        response.Push(accountProfileBase);

        return {};
    }

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        std::vector<char> profileImage(profileImageIcon->size);

        profileImageIcon->Read(skyline::span<char>(profileImage), 0);

        request.outputBuf.at(0).copy_from(profileImage);
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture(){
        const std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            return state.os->assetFileSystem->OpenFile("skyline-logo.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}


Review comment (suggestion):
```suggestion
        std::string profilePicturePath{*state.settings->profilePictureValue};
```

Code after:

        return {};
    }

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};

        profileImageIcon->Read(span(request.outputBuf.at(0)).first(profileImageIcon->size), 0);
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture() {
        std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            // If we can't find the profile picture then just return the placeholder profile picture
            return state.os->assetFileSystem->OpenFile("profile_picture.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}
Repository: skyline-emu/skyline
File: app/src/main/cpp/skyline/services/account/IProfile.cpp (C++)

Code before:
    }

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        std::vector<char> profileImage(profileImageIcon->size);

        profileImageIcon->Read(skyline::span<char>(profileImage), 0);

        request.outputBuf.at(0).copy_from(profileImage);
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture(){
        const std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            return state.os->assetFileSystem->OpenFile("skyline-logo.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}


Review comment (suggestion):
```suggestion
        if (fd < 0)
            // If we can't find the profile picture then just return the placeholder profile picture
            return state.os->assetFileSystem->OpenFile("skyline-logo.jpeg");
```

Code after:

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};

        profileImageIcon->Read(span(request.outputBuf.at(0)).first(profileImageIcon->size), 0);
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture() {
        std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            // If we can't find the profile picture then just return the placeholder profile picture
            return state.os->assetFileSystem->OpenFile("profile_picture.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}
Repository: skyline-emu/skyline
File: app/src/main/cpp/skyline/services/account/IProfile.cpp (C++)

Code before:
    }

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        std::vector<char> profileImage(profileImageIcon->size);

        profileImageIcon->Read(skyline::span<char>(profileImage), 0);

        request.outputBuf.at(0).copy_from(profileImage);
        response.Push<u32>(profileImageIcon->size);

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture(){
        const std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            return state.os->assetFileSystem->OpenFile("skyline-logo.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}


Review comment (suggestion):
I'd say the asset should just be named `profile_picture.jpeg` rather than explicitly `skyline-logo.jpeg`, you can also change references from "Skyline's icon" in comments to "the default profile picture".

Code after:

    Result IProfile::GetImageSize(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    Result IProfile::LoadImage(type::KSession &session, ipc::IpcRequest &request, ipc::IpcResponse &response) {
        std::shared_ptr<vfs::Backing> profileImageIcon{GetProfilePicture()};

        profileImageIcon->Read(span(request.outputBuf.at(0)).first(profileImageIcon->size), 0);
        response.Push(static_cast<u32>(profileImageIcon->size));

        return {};
    }

    std::shared_ptr<vfs::Backing> IProfile::GetProfilePicture() {
        std::string profilePicturePath{*state.settings->profilePictureValue};
        int fd{open((profilePicturePath).c_str(), O_RDONLY)};
        if (fd < 0)
            // If we can't find the profile picture then just return the placeholder profile picture
            return state.os->assetFileSystem->OpenFile("profile_picture.jpeg");
        else
            return std::make_shared<vfs::OsBacking>(fd, true);
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/MainViewModel.kt (Kotlin)

Code before:
                MainState.Loaded(HashMap())
            } else {
                try {
                    KeyReader.importFromLocation(context, searchLocation)
                    val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
                    romElements.toFile(romsFile)
                    MainState.Loaded(romElements)
                } catch (e : Exception) {
                    Log.w(TAG, "Ran into exception while saving: ${e.message}")
                    MainState.Error(e)
                }
            }
        }
    }

    /**
     * This checks if the roms have changed since the last time they were loaded and if so it reloads them
     */
    fun checkRomHash(context : Context, searchLocation : Uri, systemLanguage : Int) {
        CoroutineScope(Dispatchers.IO).launch {
            val oldHashFile = File(getApplication<SkylineApplication>().filesDir.canonicalPath + "/roms.txt")
            val oldHash = if (oldHashFile.exists()) oldHashFile.readText().toIntOrNull() else null
            val romElements = romProvider.loadRoms(searchLocation, systemLanguage).values.flatten().toList()
            val newHash = romElements.map { it.name }.toSet().hashCode()
            if (newHash != oldHash) {
                loadRoms(context, false, searchLocation, systemLanguage)
                oldHashFile.writeText(newHash.toString())
            }
        }
    }
}


Review comment (suggestion):
Since you've already have loaded the new ROMs, there's no need to request a new load, you can just assign them here. Since this is a `LiveData` object, changes will be automatically propagated to the game view.
```suggestion
                state = MainState.Loaded(romElements)
```

Code after:
                MainState.Loaded(HashMap())
            } else {
                try {
                    KeyReader.importFromLocation(context, searchLocation)
                    val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
                    romElements.toFile(romsFile)
                    MainState.Loaded(romElements)
                } catch (e : Exception) {
                    Log.w(TAG, "Ran into exception while saving: ${e.message}")
                    MainState.Error(e)
                }
            }
        }
    }

    /**
     * This checks if the roms have changed since the last time they were loaded and if so it reloads them
     */
    fun checkRomHash(searchLocation : Uri, systemLanguage : Int) {
        if(state !is MainState.Loaded) return
        CoroutineScope(Dispatchers.IO).launch {
            val currentHash = (state as MainState.Loaded).items.hashCode()
            val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
            val newHash = romElements.hashCode()
            if (newHash != currentHash) {
                state = MainState.Loaded(romElements)
            }
        }
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/MainViewModel.kt (Kotlin)

Code before:

            state = if (searchLocation.toString().isEmpty()) {
                @Suppress("ReplaceWithEnumMap")
                MainState.Loaded(HashMap())
            } else {
                try {
                    KeyReader.importFromLocation(context, searchLocation)
                    val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
                    romElements.toFile(romsFile)
                    MainState.Loaded(romElements)
                } catch (e : Exception) {
                    Log.w(TAG, "Ran into exception while saving: ${e.message}")
                    MainState.Error(e)
                }
            }
        }
    }

    /**
     * This checks if the roms have changed since the last time they were loaded and if so it reloads them
     */
    fun checkRomHash(context : Context, searchLocation : Uri, systemLanguage : Int) {
        CoroutineScope(Dispatchers.IO).launch {
            val oldHashFile = File(getApplication<SkylineApplication>().filesDir.canonicalPath + "/roms.txt")
            val oldHash = if (oldHashFile.exists()) oldHashFile.readText().toIntOrNull() else null
            val romElements = romProvider.loadRoms(searchLocation, systemLanguage).values.flatten().toList()
            val newHash = romElements.map { it.name }.toSet().hashCode()
            if (newHash != oldHash) {
                loadRoms(context, false, searchLocation, systemLanguage)
                oldHashFile.writeText(newHash.toString())
            }
        }
    }
}


Review comment (suggestion):
Keep the full `HashMap` object here, see the other comment.
```suggestion
            val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
```

Code after:

            state = if (searchLocation.toString().isEmpty()) {
                @Suppress("ReplaceWithEnumMap")
                MainState.Loaded(HashMap())
            } else {
                try {
                    KeyReader.importFromLocation(context, searchLocation)
                    val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
                    romElements.toFile(romsFile)
                    MainState.Loaded(romElements)
                } catch (e : Exception) {
                    Log.w(TAG, "Ran into exception while saving: ${e.message}")
                    MainState.Error(e)
                }
            }
        }
    }

    /**
     * This checks if the roms have changed since the last time they were loaded and if so it reloads them
     */
    fun checkRomHash(searchLocation : Uri, systemLanguage : Int) {
        if(state !is MainState.Loaded) return
        CoroutineScope(Dispatchers.IO).launch {
            val currentHash = (state as MainState.Loaded).items.hashCode()
            val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
            val newHash = romElements.hashCode()
            if (newHash != currentHash) {
                state = MainState.Loaded(romElements)
            }
        }
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/MainViewModel.kt (Kotlin)

Code before:
            state = if (searchLocation.toString().isEmpty()) {
                @Suppress("ReplaceWithEnumMap")
                MainState.Loaded(HashMap())
            } else {
                try {
                    KeyReader.importFromLocation(context, searchLocation)
                    val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
                    romElements.toFile(romsFile)
                    MainState.Loaded(romElements)
                } catch (e : Exception) {
                    Log.w(TAG, "Ran into exception while saving: ${e.message}")
                    MainState.Error(e)
                }
            }
        }
    }

    /**
     * This checks if the roms have changed since the last time they were loaded and if so it reloads them
     */
    fun checkRomHash(context : Context, searchLocation : Uri, systemLanguage : Int) {
        CoroutineScope(Dispatchers.IO).launch {
            val oldHashFile = File(getApplication<SkylineApplication>().filesDir.canonicalPath + "/roms.txt")
            val oldHash = if (oldHashFile.exists()) oldHashFile.readText().toIntOrNull() else null
            val romElements = romProvider.loadRoms(searchLocation, systemLanguage).values.flatten().toList()
            val newHash = romElements.map { it.name }.toSet().hashCode()
            if (newHash != oldHash) {
                loadRoms(context, false, searchLocation, systemLanguage)
                oldHashFile.writeText(newHash.toString())
            }
        }
    }
}


Review comment (suggestion):
```suggestion
            val newHash = romElements.hashCode()
```

Code after:
            state = if (searchLocation.toString().isEmpty()) {
                @Suppress("ReplaceWithEnumMap")
                MainState.Loaded(HashMap())
            } else {
                try {
                    KeyReader.importFromLocation(context, searchLocation)
                    val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
                    romElements.toFile(romsFile)
                    MainState.Loaded(romElements)
                } catch (e : Exception) {
                    Log.w(TAG, "Ran into exception while saving: ${e.message}")
                    MainState.Error(e)
                }
            }
        }
    }

    /**
     * This checks if the roms have changed since the last time they were loaded and if so it reloads them
     */
    fun checkRomHash(searchLocation : Uri, systemLanguage : Int) {
        if(state !is MainState.Loaded) return
        CoroutineScope(Dispatchers.IO).launch {
            val currentHash = (state as MainState.Loaded).items.hashCode()
            val romElements = romProvider.loadRoms(searchLocation, systemLanguage)
            val newHash = romElements.hashCode()
            if (newHash != currentHash) {
                state = MainState.Loaded(romElements)
            }
        }
    }
}
Repository: sm64pc/sm64pc
File: src/game/bettercamera.inc.h (C/C++)

Code before:
    print_text_fmt_int(32,160,"X %d",gMarioState->pos[0]);
    print_text_fmt_int(32,144,"Y %d",gMarioState->pos[1]);
    print_text_fmt_int(32,128,"Z %d",gMarioState->pos[2]);
    print_text_fmt_int(32,112,"FLAGS %d",newcam_modeflags);
    print_text_fmt_int(180,112,"INTM %d",newcam_intendedmode);
    print_text_fmt_int(32,96,"TILT UP %d",newcam_tilt_acc);
    print_text_fmt_int(32,80,"YAW UP %d",newcam_yaw_acc);
    print_text_fmt_int(32,64,"YAW %d",newcam_yaw);
    print_text_fmt_int(32,48,"TILT  %d",newcam_tilt);
    print_text_fmt_int(32,32,"DISTANCE %d",newcam_distance);
}

static s16 newcam_adjust_value(s16 var, s16 val)
{
    if (val > 0)
    {
        var += val;
        if (var > max)
            var = max;
    }
    else if (val < 0)
    {
        var += val;
        if (var < max)
            var = max;
    }
    return var;
}

static f32 newcam_approach_float(f32 var, f32 val, f32 inc)
{
    if (var < val)
        return min(var + inc, val);
        else
        return max(var - inc, val);
}

static s16 newcam_approach_s16(s16 var, s16 val, s16 inc)
{
    if (var < val)
        return max(var + inc, val);
        else
        return min(var - inc, val);
}

static int ivrt(u8 axis)
{
    if (axis == 0)
    {
        if (newcam_invertX == 0)
            return -1;

Review comment (suggestion):
`error: 'max' undeclared (first use in this function); did you mean 'fmax'?`

Code after:
    print_text_fmt_int(32,160,"X %d",gMarioState->pos[0]);
    print_text_fmt_int(32,144,"Y %d",gMarioState->pos[1]);
    print_text_fmt_int(32,128,"Z %d",gMarioState->pos[2]);
    print_text_fmt_int(32,112,"FLAGS %d",newcam_modeflags);
    print_text_fmt_int(180,112,"INTM %d",newcam_intendedmode);
    print_text_fmt_int(32,96,"TILT UP %d",newcam_tilt_acc);
    print_text_fmt_int(32,80,"YAW UP %d",newcam_yaw_acc);
    print_text_fmt_int(32,64,"YAW %d",newcam_yaw);
    print_text_fmt_int(32,48,"TILT  %d",newcam_tilt);
    print_text_fmt_int(32,32,"DISTANCE %d",newcam_distance);
}

static s16 newcam_adjust_value(s16 var, s16 val, s8 max)
{
    if (val > 0)
    {
        var += val;
        if (var > max)
            var = max;
    }
    else if (val < 0)
    {
        var += val;
        if (var < max)
            var = max;
    }
    return var;
}

static f32 newcam_approach_float(f32 var, f32 val, f32 inc)
{
    if (var < val)
        return min(var + inc, val);
        else
        return max(var - inc, val);
}

static s16 newcam_approach_s16(s16 var, s16 val, s16 inc)
{
    if (var < val)
        return max(var + inc, val);
        else
        return min(var - inc, val);
}

static int ivrt(u8 axis)
{
    if (axis == 0)
    {
        if (newcam_invertX == 0)
            return -1;