* Re: [Powertop] [PATCH v2 1/2] Updates to support Android platform
@ 2012-09-25 6:11 Arjan van de Ven
0 siblings, 0 replies; 6+ messages in thread
From: Arjan van de Ven @ 2012-09-25 6:11 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]
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(a)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
?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Powertop] [PATCH v2 1/2] Updates to support Android platform
@ 2012-09-25 15:25 Chris Ferron
0 siblings, 0 replies; 6+ messages in thread
From: Chris Ferron @ 2012-09-25 15:25 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 5316 bytes --]
On 09/24/2012 10:34 PM, Rajagopal Venkat wrote:
> On 24 September 2012 21:27, Chris Ferron <chris.e.ferron(a)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(a)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.
>>
> 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 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"
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Powertop] [PATCH v2 1/2] Updates to support Android platform
@ 2012-09-25 5:45 Arjan van de Ven
0 siblings, 0 replies; 6+ messages in thread
From: Arjan van de Ven @ 2012-09-25 5:45 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]
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(a)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
?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Powertop] [PATCH v2 1/2] Updates to support Android platform
@ 2012-09-25 5:34 Rajagopal Venkat
0 siblings, 0 replies; 6+ messages in thread
From: Rajagopal Venkat @ 2012-09-25 5:34 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 4339 bytes --]
On 24 September 2012 21:27, Chris Ferron <chris.e.ferron(a)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(a)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.
>
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 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"
>
>
--
Regards,
Rajagopal
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Powertop] [PATCH v2 1/2] Updates to support Android platform
@ 2012-09-24 15:57 Chris Ferron
0 siblings, 0 replies; 6+ messages in thread
From: Chris Ferron @ 2012-09-24 15:57 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 3339 bytes --]
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(a)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");
> }
> 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"
^ permalink raw reply [flat|nested] 6+ messages in thread* [Powertop] [PATCH v2 1/2] Updates to support Android platform
@ 2012-09-24 13:28 Rajagopal Venkat
0 siblings, 0 replies; 6+ messages in thread
From: Rajagopal Venkat @ 2012-09-24 13:28 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 2949 bytes --]
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(a)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"
--
1.7.11.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-25 15:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-25 6:11 [Powertop] [PATCH v2 1/2] Updates to support Android platform Arjan van de Ven
-- strict thread matches above, loose matches on Subject: below --
2012-09-25 15:25 Chris Ferron
2012-09-25 5:45 Arjan van de Ven
2012-09-25 5:34 Rajagopal Venkat
2012-09-24 15:57 Chris Ferron
2012-09-24 13:28 Rajagopal Venkat
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.