[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