Join declarations and assignments #13025
Conversation
| SplitPathResult result; | ||
|
|
||
| result = {"/shared1", "00000042.app"}; | ||
| SplitPathResult result = {"/shared1", "00000042.app"}; |
There was a problem hiding this comment.
This reduces readability of the test imo.
There was a problem hiding this comment.
What is less clear about this?
There was a problem hiding this comment.
This reduces readability of the test imo.
I also think the same, as result will be assigned different values depending on the test. Leaving the declaration (without initialization) at the beginning makes the intent clearer that this variable might be reused with new values.
| std::unique_ptr<StagingBuffer> temp_buffer = StagingBuffer::Create( | ||
| STAGING_BUFFER_TYPE_UPLOAD, upload_size, VK_BUFFER_USAGE_TRANSFER_SRC_BIT); |
There was a problem hiding this comment.
This affects object lifetime, not sure whether that matters but just noting...
There was a problem hiding this comment.
In fact, I'm pretty sure this breaks the code, looking at this in more detail. upload_buffer keeps a pointer to the now out-of-scope temp_buffer->GetBuffer().
There was a problem hiding this comment.
I don't see how this breaks anything
There was a problem hiding this comment.
As @AdmiralCurtiss said, upload_buffer will point to a destroyed object (held by the unique_ptr) when exiting the current else statement (i.e. which will destroy temp_buffer declared in this scope).
There was a problem hiding this comment.
Since it wasn't caught when building I didn't think it would be an issue.
Do you have any tips for how to avoid this in the future? (Other than switching to Rust :P)
|
|
||
| if (is_down) | ||
| { | ||
| utf8 = key_event->text().toUtf8(); | ||
| QByteArray utf8 = key_event->text().toUtf8(); |
There was a problem hiding this comment.
Same problem here. utf8 goes out of scope but chars keeps a pointer to it.
There was a problem hiding this comment.
I don't see what's wrong here either
There was a problem hiding this comment.
When exiting the if (is_down) statement, utf8 will be destroyed and chars will become a dangling pointer pointing to destroyed data (see chars = utf8.constData()).
|
Stopping here because I must ask: Did you even check these refactorings? |
Source/Core/Core/PowerPC/GDBStub.cpp
Outdated
| @@ -923,16 +907,11 @@ static void HandleAddBreakpoint() | |||
|
|
|||
| static void HandleRemoveBreakpoint() | |||
| { | |||
| u32 type, addr, len, i; | |||
| u32 type(Hex2char(s_cmd_bfr[1])), addr(0), len(0), i(3); | |||
There was a problem hiding this comment.
Would rather the declaration and assignment of addr, len, and i be moved down to where it is used (where the assignment was previously). Also, why suddenly parenthesis for initialization?
There was a problem hiding this comment.
I've seen both parentheses and equal signs used for initialization in the codebase and I didn't see any pattern for when to choose which.
I probably edited this part right after some part which used parentheses.
tygyh
force-pushed
the
Join-declarations-and-assignments
branch
from
286dcf8
to
300158a
Compare
tygyh
force-pushed
the
Join-declarations-and-assignments
branch
from
300158a
to
69fcf27
Compare
No description provided.