On 09/24/2012 10:34 PM, Rajagopal Venkat wrote:
On 24 September 2012 21:27, Chris Ferron chris.e.ferron@linux.intel.com wrote:
On 09/24/2012 06:28 AM, Rajagopal Venkat wrote:
This patch adds following minor changes to prepare powertop to support Android platform.
- Add missing HAVE_CONFIG_H conditional check.
- remove un-used ethtool_cmd_speed_set and ethtool_cmd_speed
functions.
- Minimize dependency on exception handling in catch blocks.
These changes will not affect powertop functionality.
Signed-off-by: Rajagopal Venkat rajagopal.venkat@linaro.org
src/devices/ahci.cpp | 4 ++-- src/devices/alsa.cpp | 4 ++-- src/devices/network.cpp | 16 ---------------- src/main.cpp | 2 ++ 4 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp index 1fe39c7..67ce06e 100644 --- a/src/devices/ahci.cpp +++ b/src/devices/ahci.cpp @@ -170,7 +170,7 @@ void ahci::start_measurement(void) file.close(); } catch (std::ios_base::failure &c) {
fprintf(stderr, "%s\n", c.what());
fprintf(stderr, "Failed to start measurement for ahcidevice\n");
adding addition message here is acceptable, but eliminating the information from the catches error is not.
As discussed in first patch set, android doesn't support exception handling. This is the reason powertop had DISABLE_TRYCATCH conditional macro which is removed in recent commit.
The patch 2/2 adds stubs for exception handling
#define try if (true) #define catch(x) if (false)
With this, fprintf(stderr, "%s\n", c.what()); in catch block throws undefined reference to c. So added message instead of c.what().
Any better ways of adding stubs are welcome.
So it is my opinion that this is a bug for deficiency's in Android, and one that is not appropriate to work around in this core tool. Any patch/fix/workaround should be submitted to the people maintaining Android. The reason the DISABLE_TRYCATCH was removed was to eliminate any non generic need for defines within the tool. We are actively trying to eliminate and discouraging distribution specific code within the core PowerTOP source. There is some flex ability for example your stub header for android.
My current suggestion is to re-submit your patches minus the Android workarounds for C++ try-catches, and then work this issues with the fine people at google.
PowerTOP focus should be on platforms, architectures, and the Linux kernel, not distributions.
-C
} }@@ -203,7 +203,7 @@ void ahci::end_measurement(void) file.close(); } catch (std::ios_base::failure &c) {
fprintf(stderr, "%s\n", c.what());
fprintf(stderr, "Failed to end measurement for ahcidevice\n"); } if (end_active < start_active) end_active = start_active; diff --git a/src/devices/alsa.cpp b/src/devices/alsa.cpp index 4f5d3f9..a67780c 100644 --- a/src/devices/alsa.cpp +++ b/src/devices/alsa.cpp @@ -104,7 +104,7 @@ void alsa::start_measurement(void) file.close(); } catch (std::ios_base::failure &c) {
fprintf(stderr, "%s\n", c.what());
fprintf(stderr, "Failed to start measurement for alsadevice\n"); } } @@ -130,7 +130,7 @@ void alsa::end_measurement(void) file.close(); } catch (std::ios_base::failure &c) {
fprintf(stderr, "%s\n", c.what());
fprintf(stderr, "Failed to end measurement for alsadevice\n"); } p = (end_active - start_active) / (0.001 + end_active + end_inactive - start_active - start_inactive) * 100.0; diff --git a/src/devices/network.cpp b/src/devices/network.cpp index b8a5c9c..ed9d7aa 100644 --- a/src/devices/network.cpp +++ b/src/devices/network.cpp @@ -55,22 +55,6 @@ extern "C" { static map<string, class network *> nics; -#ifdef DISABLE_TRYCATCH
-static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
__u32 speed)-{
ep->speed = (__u16)speed;ep->speed_hi = (__u16)(speed >> 16);-}
-static inline __u32 ethtool_cmd_speed(struct ethtool_cmd *ep) -{
return (ep->speed_hi << 16) | ep->speed;-}
-#endif static void do_proc_net_dev(void) { diff --git a/src/main.cpp b/src/main.cpp index 1815075..dc49dba 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -42,7 +42,9 @@ #include "perf/perf.h" #include "perf/perf_bundle.h" #include "lib.h" +#ifdef HAVE_CONFIG_H #include "../config.h" +#endif #include "devices/device.h"