stella.stamenova added inline comments.
================ Comment at: lldb/source/Commands/CommandObjectMemory.cpp:1701 } else { if (command.GetArgumentCount() == 1) { auto load_addr_str = command[0].ref(); ---------------- Actually, I would change the logic here a little bit to make it easier to read. Right now it is: ``` if (argc > 1 || ... ) { } else { if (GetArgumentCount() == 1) { } ... } ``` It should be: ``` if (argc > 1 || ... ) { } else if (argc == 1) { //since argc already has the value of GetArgumentCount() } if (result.Succeeded()) { ... } ``` This will make the function more readable, fixing the bug that you found, preserving most of its logic and keeping the single return. ================ Comment at: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp:420 } else { error.SetError(::GetLastError(), eErrorTypeWin32); LLDB_LOG(log, ---------------- This GetLastError call is not guaranteed to return the same error as above since the error could be cleared by a call to GetLastError. You should instead do: ``` if (result == 0) { DWORD lastError = ::GetLastError(); if (lastError == ERROR_INVALID_PARAMETER) { } else { } } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88229/new/ https://reviews.llvm.org/D88229 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits