Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move variables to inner scope - DolphinQT #13190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tygyh
Copy link
Contributor

@tygyh tygyh commented Nov 16, 2024

Partial replacement of #13026

@tygyh tygyh force-pushed the Move-to-inner-scope-DolphinQT branch from 0c36f88 to 04bc072 Compare November 16, 2024 22:10
@mitaclaw
Copy link
Contributor

@dolphin-emu-bot rebuild

@tygyh tygyh changed the title Move variables to inner scope Move variables to inner scope - DolphinQT Nov 18, 2024
@tygyh tygyh requested a review from JosJuice November 20, 2024 07:02

if (is_down)
{
utf8 = key_event->text().toUtf8();
QByteArray utf8 = key_event->text().toUtf8();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an object lifetime issue. With this change, const char* chars will still contain the result of utf8.constData() after QByteArray utf8 goes out of scope. This is why the code was written the way it was before. This is not the first time your automated changes have produced erroneous code like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants