tberghammer accepted this revision.
tberghammer added a comment.
This revision is now accepted and ready to land.
Looks good with a few nits inline
================
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:418
@@ +417,3 @@
+bool
+ABISysV_arm::IsArmHardFloat (Thread *thread) const
+{
----------------
(nit): I would suggest to pass in the thread by reference because the function
have no meaning with a NULL thread
================
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:427
@@ +426,3 @@
+ const ArchSpec &arch (process_sp->GetTarget().GetArchitecture());
+ if (arch.GetFlags() == ArchSpec::eARM_abi_hard_float)
+ {
----------------
tberghammer wrote:
> (nit): You can write it much simpler like this:
>
> ```
> return (arch.GetFlags() & ArchSpec::eARM_abi_hard_float) != 0;
> ```
This will break if we add a new (independent) architecture flag for arm in the
future. Please only check if the given bit is set instead of comparing for
equality.
================
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:427-430
@@ +426,6 @@
+ const ArchSpec &arch (process_sp->GetTarget().GetArchitecture());
+ if (arch.GetFlags() == ArchSpec::eARM_abi_hard_float)
+ {
+ is_armhf = true;
+ }
+ }
----------------
(nit): You can write it much simpler like this:
```
return (arch.GetFlags() & ArchSpec::eARM_abi_hard_float) != 0;
```
================
Comment at: source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:539
@@ +538,3 @@
+
+ if (!IsArmHardFloat(&thread))
+ {
----------------
(nit); Please revert the order of the if and the else case so you don't have to
negate the condition
http://reviews.llvm.org/D16627
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
