[Lldb-commits] [PATCH] D24331: Fix about a dozen compile warnings
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 8 00:11:16 PDT 2016
zturner added inline comments.
================
Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1462-1467
@@ -1461,8 +1461,8 @@
const uint32_t segment_permissions =
((load_cmd.initprot & VM_PROT_READ) ? ePermissionsReadable
- : 0) |
+ : 0u) |
((load_cmd.initprot & VM_PROT_WRITE) ? ePermissionsWritable
- : 0) |
+ : 0u) |
((load_cmd.initprot & VM_PROT_EXECUTE) ? ePermissionsExecutable
- : 0);
+ : 0u);
----------------
I find this code pretty ugly because of how the line breaks. Can you change this to:
```
uint32_t segment_permissions = 0;
if (load_cmd.initprot & VM_PROT_READ)
segment_permissions |= ePermissionsReadable;
if (load_cmd.initprot & VM_PROT_WRITE)
segment_permissions |= ePermissionsWritable;
if (load_cmd.initprot & VM_PROT_EXECUTE)
segment_permissions |= ePermissionsExecutable;
```
or, alternatively:
```
// global scope
static uint32_t translateSegmentPermissions(uint32_t initprot) {
return ((initprot & VM_PROT_READ) ? ePermissionsReadable : 0u) |
(initprot & VM_PROT_WRITE) ? ePermissionsWritable : 0u) |
(initprot & VM_PROT_EXECUTE) ? ePermissionsExecutable : 0u);
}
// at this location
const uint32_t segment_permissions = translateSegmentPermissions(load_cmd.initprot);
```
================
Comment at: source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:521
@@ -520,3 +520,3 @@
const uint64_t uval = PyLong_AsUnsignedLongLong(m_py_obj);
- result = *((int64_t *)&uval);
+ result = *((const int64_t *)&uval);
}
----------------
Am I missing something? Why isn't this just `result = static_cast<int64_t>(uval);`
================
Comment at: source/Target/StackFrame.cpp:1427
@@ -1423,3 +1426,3 @@
- if (offset >= pointee->GetByteSize()) {
+ if (offset > 0 && uint64_t(offset) >= pointee->GetByteSize()) {
int64_t index = offset / pointee->GetByteSize();
----------------
Probably only of theoretical interest since I don't think `pointee->GetByteSize()` can ever return 0, but maybe this should be `offset >= 0`
================
Comment at: source/Target/StackFrame.cpp:1593
@@ -1589,3 +1592,3 @@
Instruction::Operand *register_operand = nullptr;
Instruction::Operand *origin_operand = nullptr;
----------------
How about just delete this variable instead?
https://reviews.llvm.org/D24331
More information about the lldb-commits
mailing list