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 ahci device\n"); }
} @@ -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 ahci device\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 alsa device\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 alsa device\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"
This patch adds stubs for features that are not supported by Andriod. An header file which defines all stubs is included only for Android builds.
Signed-off-by: Rajagopal Venkat rajagopal.venkat@linaro.org --- Android.mk | 33 ++++++++++++++++++++++----- src/android_stubs.h | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.h | 4 ++++ 3 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 src/android_stubs.h
diff --git a/Android.mk b/Android.mk index a52ecfd..081f470 100644 --- a/Android.mk +++ b/Android.mk @@ -1,17 +1,36 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS)
+LOCAL_MODULE := powertop + LOCAL_MODULE_TAGS := debug LOCAL_SHARED_LIBRARIES := libstlport \ libnl \ libpci \ - libtraceevnet \ -LOCAL_MODULE := powertop + +LOCAL_STATIC_LIBRARIES := libncurses + +CSSTOH_SOURCE := $(LOCAL_PATH)/src/csstoh.c +POWERTOP_CSS_SOURCE := $(LOCAL_PATH)/src/powertop.css +GEN_CSSTOH := $(LOCAL_PATH)/src/csstoh +GEN_CSS_H := $(LOCAL_PATH)/src/css.h +$(GEN_CSS_H): + $(CC) -o $(GEN_CSSTOH) $(CSSTOH_SOURCE) + ./$(GEN_CSSTOH) $(POWERTOP_CSS_SOURCE) $@ + +LOCAL_GENERATED_SOURCES += $(GEN_CSS_H)
#LOCAL_CFLAGS += -Wall -O2 -g -fno-omit-frame-pointer -fstack-protector -Wshadow -Wformat -D_FORTIFY_SOURCE=2 #LOCAL_CPPFLAGS += -Wall -O2 -g -fno-omit-frame-pointer
-LOCAL_C_INCLUDES += external/stlport/stlport/ external/stlport/stlport/stl external/stlport/stlport/using/h/ bionic external/libnl/include/ +LOCAL_C_INCLUDES += external/stlport/stlport/ \ + external/stlport/stlport/stl \ + external/stlport/stlport/using/h/ \ + bionic \ + external/libnl/include/ \ + external/ncurses/include \ + external/elfutils/bionic-fixup \ + $(LOCAL_PATH)/src
LOCAL_SRC_FILES += \ src/parameters/parameters.cpp \ @@ -21,10 +40,11 @@ LOCAL_SRC_FILES += \ src/process/work.cpp \ src/process/process.cpp \ src/process/timer.cpp \ - src/process/device.cpp \ + src/process/processdevice.cpp \ src/process/interrupt.cpp \ src/process/do_process.cpp \ src/cpu/intel_cpus.cpp \ + src/cpu/intel_gpu.cpp \ src/cpu/cpu.cpp \ src/cpu/cpu_linux.cpp \ src/cpu/cpudevice.cpp \ @@ -33,20 +53,21 @@ LOCAL_SRC_FILES += \ src/cpu/abstract_cpu.cpp \ src/measurement/measurement.cpp \ src/measurement/acpi.cpp \ + src/measurement/sysfs.cpp \ src/measurement/extech.cpp \ src/measurement/power_supply.cpp \ src/display.cpp \ src/report.cpp \ src/main.cpp \ src/tuning/tuning.cpp \ - src/tuning/usb.cpp \ + src/tuning/tuningusb.cpp \ src/tuning/bluetooth.cpp \ src/tuning/ethernet.cpp \ src/tuning/runtime.cpp \ src/tuning/iw.c \ src/tuning/iw.h \ src/tuning/tunable.cpp \ - src/tuning/sysfs.cpp \ + src/tuning/tuningsysfs.cpp \ src/tuning/cpufreq.cpp \ src/tuning/wifi.cpp \ src/perf/perf_bundle.cpp \ diff --git a/src/android_stubs.h b/src/android_stubs.h new file mode 100644 index 0000000..ed37c0e --- /dev/null +++ b/src/android_stubs.h @@ -0,0 +1,65 @@ +#include <linux/ethtool.h> +#include <sys/socket.h> +#include <string.h> +#include <wchar.h> + + +/* Android doesn't provide locale support in its C and C++ + * runtime. Handled at higher level in application stack. + * So define stubs for gettext funtions used. + */ +#define PACKAGE 0 +#define LOCALEDIR 0 +#define bindtextdomain(x, y) +#define textdomain(x) +#define gettext(x) (x) + +/* Android C++ new operator does not throw exception on failure */ +#define set_new_handler(x) + +/* define stubs for C++ exception handling */ +#define try if (true) +#define catch(x) if (false) + +/* Define __NR_perf_event_open if not already defined */ +#if __arm__ +#ifndef __NR_perf_event_open +#define __NR_perf_event_open 364 +#endif +#endif + +/* + * bionic libc mbstowcs version returns zero when max parameter + * is zero, resulting infinite loops in powertop source. Add + * mbstowcs wrapper to fix it. + */ +namespace pandroid { + extern "C" inline size_t mbstowcs(wchar_t *dst, + const char *src, size_t len) + { + return ::mbstowcs(dst, src, ::strlen(src)); + } +} + +#define mbstowcs(dst, src, len) pandroid::mbstowcs(dst, src, len) + +/* Implement missing functions */ +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; +} + +static inline char *strchrnul(const char *s, int c) +{ + while (*s && (*s != c)) + s++; + return (char *)s; +} diff --git a/src/lib.h b/src/lib.h index 8cf4632..6772904 100644 --- a/src/lib.h +++ b/src/lib.h @@ -33,6 +33,10 @@ #include "config.h" #endif
+#ifdef __ANDROID__ +#include "android_stubs.h" +#endif + #define _(STRING) gettext(STRING)
#define POWERTOP_VERSION "v2.1"
On Mon, Sep 24, 2012 at 06:58:04PM +0530, Rajagopal Venkat wrote:
This patch adds stubs for features that are not supported by Andriod. An header file which defines all stubs is included only for Android builds.
Signed-off-by: Rajagopal Venkat rajagopal.venkat@linaro.org diff --git a/src/android_stubs.h b/src/android_stubs.h new file mode 100644 index 0000000..ed37c0e --- /dev/null +++ b/src/android_stubs.h
+/* Android C++ new operator does not throw exception on failure */ +#define set_new_handler(x)
That they fail to throw exceptions from new is no reason to disable set_new_handler, the newhandler is called by the runtime on out of memory and is intended to allow the user to try fixing the issue. This is true for the noexcept version as well. Is this yet another incompatibility?
+/* define stubs for C++ exception handling */ +#define try if (true) +#define catch(x) if (false)
If you should do this then I think it should be spelled
#define try if (true) #define catch else if (false)
in order to not break
if (condition) try { } catch(type variable) { }
but it still breaks the syntax for it which can be shown by simply adding an else clause to the if statement.
Oh, and furthermore I consider that Android needs a C++ compiler.
+/* Define __NR_perf_event_open if not already defined */ +#if __arm__ +#ifndef __NR_perf_event_open +#define __NR_perf_event_open 364 +#endif +#endif
+/*
- bionic libc mbstowcs version returns zero when max parameter
- is zero, resulting infinite loops in powertop source. Add
- mbstowcs wrapper to fix it.
- */
+namespace pandroid {
- extern "C" inline size_t mbstowcs(wchar_t *dst,
const char *src, size_t len)- {
return ::mbstowcs(dst, src, ::strlen(src));- }
+}
+#define mbstowcs(dst, src, len) pandroid::mbstowcs(dst, src, len)
Still broken. If dst isn't a NULL pointer then len is the limit on the length of the destination buffer. In throwing away this you open up for stack smashing attacks.
/MF
On (09/27/12 00:09), Magnus Fromreide wrote:
That they fail to throw exceptions from new is no reason to disable set_new_handler, the newhandler is called by the runtime on out of memory and is intended to allow the user to try fixing the issue. This is true for the noexcept version as well. Is this yet another incompatibility?
right. the funny thing is, guess what, gcc actually has "#ifdef __EXCEPTIONS" within operator new()
44 _GLIBCXX_WEAK_DEFINITION void * 45 operator new (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc) 46 { 47 void *p; 48 49 /* malloc (0) is unpredictable; avoid it. */ 50 if (sz == 0) 51 sz = 1; 52 p = (void *) malloc (sz); 53 while (p == 0) 54 { 55 new_handler handler = __new_handler; 56 if (! handler) 57 #ifdef __EXCEPTIONS 58 throw bad_alloc(); 59 #else 60 std::abort(); 61 #endif 62 handler (); 63 p = (void *) malloc (sz); 64 } 65 66 return p; 67 }
It tourned out, that __EXCEPTIONS with us (for operator new and STL) since 2001 "2001-02-15 Benjamin Kosnik bkoz@redhat.com"
So, I guess this is how Google has came up with the idea of C++ w/o exceptions.
Wow.
-ss
+/* define stubs for C++ exception handling */ +#define try if (true) +#define catch(x) if (false)
If you should do this then I think it should be spelled
#define try if (true) #define catch else if (false)
in order to not break
if (condition) try { } catch(type variable) { }
but it still breaks the syntax for it which can be shown by simply adding an else clause to the if statement.
Oh, and furthermore I consider that Android needs a C++ compiler.
+/* Define __NR_perf_event_open if not already defined */ +#if __arm__ +#ifndef __NR_perf_event_open +#define __NR_perf_event_open 364 +#endif +#endif
+/*
- bionic libc mbstowcs version returns zero when max parameter
- is zero, resulting infinite loops in powertop source. Add
- mbstowcs wrapper to fix it.
- */
+namespace pandroid {
- extern "C" inline size_t mbstowcs(wchar_t *dst,
const char *src, size_t len)- {
return ::mbstowcs(dst, src, ::strlen(src));- }
+}
+#define mbstowcs(dst, src, len) pandroid::mbstowcs(dst, src, len)
Still broken. If dst isn't a NULL pointer then len is the limit on the length of the destination buffer. In throwing away this you open up for stack smashing attacks.
On 09/26/2012 04:00 PM, Sergey Senozhatsky wrote:
On (09/27/12 00:09), Magnus Fromreide wrote:
That they fail to throw exceptions from new is no reason to disable set_new_handler, the newhandler is called by the runtime on out of memory and is intended to allow the user to try fixing the issue. This is true for the noexcept version as well. Is this yet another incompatibility?
right. the funny thing is, guess what, gcc actually has "#ifdef __EXCEPTIONS" within operator new()
44 _GLIBCXX_WEAK_DEFINITION void * 45 operator new (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc) 46 { 47 void *p; 48 49 /* malloc (0) is unpredictable; avoid it. */ 50 if (sz == 0) 51 sz = 1; 52 p = (void *) malloc (sz); 53 while (p == 0) 54 { 55 new_handler handler = __new_handler; 56 if (! handler) 57 #ifdef __EXCEPTIONS 58 throw bad_alloc(); 59 #else 60 std::abort(); 61 #endif 62 handler (); 63 p = (void *) malloc (sz); 64 } 65 66 return p; 67 }
It tourned out, that __EXCEPTIONS with us (for operator new and STL) since 2001 "2001-02-15 Benjamin Kosnik bkoz@redhat.com"
So, I guess this is how Google has came up with the idea of C++ w/o exceptions.
Wow.
-ss
Well to be fair, not that I agree any-more, but years ago it was common practice to disable exceptions (via the compiler) for C++ in very specialized "REAL" "Embedded Systems". This practice was problematic in that you needed to have a will defined Error handling design, but saved space. Honestly, its been a long time since I worked on anything that has a "REAL" size requirement, so I don't see the point. On the other hand in some Embedded system The whole reason to using your own error handling design was that in a real embedded system, design considerations like, uptime, failure control and ability to eliminate undefined behaviour were KEY. So taking the time to design was already a given, and size was not a single point factor. (not arguing the C vs C++ point here BTW) So for what ever reason the decision to use C++ happens for an embedded project. Problem was C++ exception were good for insure the program didn't fail, but detailed failure handling became problematic. Sure you could avoid general failures but often especially in an embedded system you found yourself with undefined behaviour that was nearly imposable to handle. OK in general history over :)
Unfortunately this practice has been inherited still today in segments of other then embedded. Most commonly you may see this practice in specialized segments like gaming consoles. Such practices are still in valid use in such device as switches, telecommunications, avionic systems, weapon systems, medical devices, ect.
So I have an understanding of the issues, "some" of its history, and valid uses. But in my opinion this is still a bad implementation for anyone distributing a general operating system.
+/* define stubs for C++ exception handling */ +#define try if (true) +#define catch(x) if (false)
If you should do this then I think it should be spelled
#define try if (true) #define catch else if (false)
in order to not break
if (condition) try { } catch(type variable) { }
but it still breaks the syntax for it which can be shown by simply adding an else clause to the if statement.
Oh, and furthermore I consider that Android needs a C++ compiler.
+/* Define __NR_perf_event_open if not already defined */ +#if __arm__ +#ifndef __NR_perf_event_open +#define __NR_perf_event_open 364 +#endif +#endif
+/*
- bionic libc mbstowcs version returns zero when max parameter
- is zero, resulting infinite loops in powertop source. Add
- mbstowcs wrapper to fix it.
- */
+namespace pandroid {
- extern "C" inline size_t mbstowcs(wchar_t *dst,
const char *src, size_t len)- {
return ::mbstowcs(dst, src, ::strlen(src));- }
+}
+#define mbstowcs(dst, src, len) pandroid::mbstowcs(dst, src, len)
Still broken. If dst isn't a NULL pointer then len is the limit on the length of the destination buffer. In throwing away this you open up for stack smashing attacks.
PowerTop mailing list PowerTop@lists.01.org https://lists.01.org/mailman/listinfo/powertop
On (09/28/12 09:54), Chris Ferron wrote: [..]
Well to be fair, not that I agree any-more, but years ago it was common practice to disable exceptions (via the compiler) for C++ in very specialized "REAL" "Embedded Systems". This practice was problematic in that you needed to have a will defined Error handling design, but saved space. Honestly, its been a long time since I worked on anything that has a "REAL" size requirement, so I don't see the point. On the other hand in some Embedded system The whole reason to using your own error handling design was that in a real embedded system, design considerations like, uptime, failure control and ability to eliminate undefined behaviour were KEY. So taking the time to design was already a given, and size was not a single point factor. (not arguing the C vs C++ point here BTW) So for what ever reason the decision to use C++ happens for an embedded project. Problem was C++ exception were good for insure the program didn't fail, but detailed failure handling became problematic. Sure you could avoid general failures but often especially in an embedded system you found yourself with undefined behaviour that was nearly imposable to handle. OK in general history over :)
Unfortunately this practice has been inherited still today in segments of other then embedded. Most commonly you may see this practice in specialized segments like gaming consoles. Such practices are still in valid use in such device as switches, telecommunications, avionic systems, weapon systems, medical devices, ect.
So I have an understanding of the issues, "some" of its history, and valid uses. But in my opinion this is still a bad implementation for anyone distributing a general operating system.
Sure, I totally agree. Nowadays, with 4 CPU cores in a pocket, I simply don't buy "exceptions are slow" argument. If in some particular project exceptions are so common that they're able to sensibly slow down application, then the project most probably is doing something terribly wrong and mis-concept exceptions.
The tragedy is that at this point in time I believe Google will never consider using exceptions due to legacy reasons.
-ss
On 09/28/2012 10:10 AM, Sergey Senozhatsky wrote:
On (09/28/12 09:54), Chris Ferron wrote: [..]
Well to be fair, not that I agree any-more, but years ago it was common practice to disable exceptions (via the compiler) for C++ in very specialized "REAL" "Embedded Systems". This practice was problematic in that you needed to have a will defined Error handling design, but saved space. Honestly, its been a long time since I worked on anything that has a "REAL" size requirement, so I don't see the point. On the other hand in some Embedded system The whole reason to using your own error handling design was that in a real embedded system, design considerations like, uptime, failure control and ability to eliminate undefined behaviour were KEY. So taking the time to design was already a given, and size was not a single point factor. (not arguing the C vs C++ point here BTW) So for what ever reason the decision to use C++ happens for an embedded project. Problem was C++ exception were good for insure the program didn't fail, but detailed failure handling became problematic. Sure you could avoid general failures but often especially in an embedded system you found yourself with undefined behaviour that was nearly imposable to handle. OK in general history over :)
Unfortunately this practice has been inherited still today in segments of other then embedded. Most commonly you may see this practice in specialized segments like gaming consoles. Such practices are still in valid use in such device as switches, telecommunications, avionic systems, weapon systems, medical devices, ect.
So I have an understanding of the issues, "some" of its history, and valid uses. But in my opinion this is still a bad implementation for anyone distributing a general operating system.
Sure, I totally agree. Nowadays, with 4 CPU cores in a pocket, I simply don't buy "exceptions are slow" argument. If in some particular project exceptions are so common that they're able to sensibly slow down application, then the project most probably is doing something terribly wrong and mis-concept exceptions.
Agree
The tragedy is that at this point in time I believe Google will never consider using exceptions due to legacy reasons.
-ss
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 ahci device\n");
adding addition message here is acceptable, but eliminating the information from the catches error is not.
} } @@ -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 ahci device\n");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 alsa device\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 alsa device\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"
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.
} }@@ -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"
On 9/25/2012 7:34 AM, Rajagopal Venkat 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
As discussed in first patch set, android doesn't support exception handling. This
where is this in your commit log?
is the reason powertop had DISABLE_TRYCATCH conditional macro which is removed in recent commit.
Android being braindead about the C++ language should be it's own patch, not snuck in as part of another change.
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.
#define try(x) if (true) { x; #define catch(x) } else
?
On 9/25/2012 7:34 AM, Rajagopal Venkat 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
As discussed in first patch set, android doesn't support exception handling. This
where is this in your commit log?
is the reason powertop had DISABLE_TRYCATCH conditional macro which is removed in recent commit.
Android being braindead about the C++ language should be it's own patch, not snuck in as part of another change.
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.
#define try(x) if (true) { x; #define catch(x) } else
?
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"
-
Arjan van de Ven -
Chris Ferron -
Magnus Fromreide -
Rajagopal Venkat -
Sergey Senozhatsky