2:13 a.m.
Modern linux distribution forces to check result of write() call.
It emits following warnings:
CXX calibrate/powertop-calibrate.o
calibrate/calibrate.cpp: In function ‘void* burn_disk(void*)’:
calibrate/calibrate.cpp:297: warning: ignoring return value of ‘ssize_t write(int, const
void*, size_t)’, declared with attribute warn_unused_result
...
CXX measurement/powertop-extech.o
measurement/extech.cpp: In member function ‘void extech_power_meter::measure()’:
measurement/extech.cpp:290: warning: ignoring return value of ‘ssize_t write(int, const
void*, size_t)’, declared with attribute warn_unused_result
Signed-off-by: Namhyung Kim <namhyung(a)gmail.com>
---
src/calibrate/calibrate.cpp | 3 ++-
src/measurement/extech.cpp | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/calibrate/calibrate.cpp b/src/calibrate/calibrate.cpp
index 5457cf8..2d4c1af 100644
--- a/src/calibrate/calibrate.cpp
+++ b/src/calibrate/calibrate.cpp
@@ -294,7 +294,8 @@ static void *burn_disk(void *dummy)
while (!stop_measurement) {
lseek(fd, 0, SEEK_SET);
- write(fd, buffer, 64*1024);
+ if (write(fd, buffer, 64*1024))
+ /* make gcc silent */;
fdatasync(fd);
}
close(fd);
diff --git a/src/measurement/extech.cpp b/src/measurement/extech.cpp
index 4e8bf8a..b9e95ca 100644
--- a/src/measurement/extech.cpp
+++ b/src/measurement/extech.cpp
@@ -287,7 +287,8 @@ extech_power_meter::extech_power_meter(const char *extech_name)
void extech_power_meter::measure(void)
{
/* trigger the extech to send data */
- write(fd, " ", 1);
+ if (write(fd, " ", 1) < 0)
+ return;
rate = extech_read(fd);
--
1.7.9.2
2:13 a.m.
New subject: [Powertop] [PATCH 2/3] Suppress compiler warnings on system()
As system(3) requires to check its return value, make a wrapper
do_system() to suppress following warnings:
CXX tuning/powertop-bluetooth.o
tuning/bluetooth.cpp: In member function ‘virtual void bt_tunable::toggle()’:
tuning/bluetooth.cpp:179: warning: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
tuning/bluetooth.cpp:182: warning: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
CXX tuning/powertop-cpufreq.o
tuning/cpufreq.cpp: In member function ‘virtual void cpufreq_tunable::toggle()’:
tuning/cpufreq.cpp:118: warning: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
...
CXX calibrate/powertop-calibrate.o
calibrate/calibrate.cpp: In function ‘void backlight_calibration()’:
calibrate/calibrate.cpp:408: warning: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
calibrate/calibrate.cpp:410: warning: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
calibrate/calibrate.cpp: In function ‘void idle_calibration()’:
calibrate/calibrate.cpp:416: warning: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
calibrate/calibrate.cpp:418: warning: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
Signed-off-by: Namhyung Kim <namhyung(a)gmail.com>
---
src/calibrate/calibrate.cpp | 8 ++++----
src/lib.cpp | 5 +++++
src/lib.h | 1 +
src/tuning/bluetooth.cpp | 4 ++--
src/tuning/cpufreq.cpp | 2 +-
5 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/calibrate/calibrate.cpp b/src/calibrate/calibrate.cpp
index 2d4c1af..c886b73 100644
--- a/src/calibrate/calibrate.cpp
+++ b/src/calibrate/calibrate.cpp
@@ -406,17 +406,17 @@ static void backlight_calibration(void)
sleep(1);
}
printf(_("Calibrating idle\n"));
- system("DISPLAY=:0 /usr/bin/xset dpms force off");
+ do_system("DISPLAY=:0 /usr/bin/xset dpms force off");
one_measurement(15, NULL);
- system("DISPLAY=:0 /usr/bin/xset dpms force on");
+ do_system("DISPLAY=:0 /usr/bin/xset dpms force on");
}
static void idle_calibration(void)
{
printf(_("Calibrating idle\n"));
- system("DISPLAY=:0 /usr/bin/xset dpms force off");
+ do_system("DISPLAY=:0 /usr/bin/xset dpms force off");
one_measurement(15, NULL);
- system("DISPLAY=:0 /usr/bin/xset dpms force on");
+ do_system("DISPLAY=:0 /usr/bin/xset dpms force on");
}
diff --git a/src/lib.cpp b/src/lib.cpp
index 9838c0b..ccba414 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -441,3 +441,8 @@ int get_user_input(char *buf, unsigned sz)
/* to distinguish between getnstr error and empty line */
return ret || strlen(buf);
}
+
+int do_system(const char *command)
+{
+ return system(command); /* make gcc slient */
+}
diff --git a/src/lib.h b/src/lib.h
index 209421a..7002f14 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -73,5 +73,6 @@ typedef void (*callback)(const char*);
extern void process_directory(const char *d_name, callback fn);
extern int utf_ok;
extern int get_user_input(char *buf, unsigned sz);
+extern int do_system(const char *cmd);
#endif
diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
index e0bdf12..b717610 100644
--- a/src/tuning/bluetooth.cpp
+++ b/src/tuning/bluetooth.cpp
@@ -176,10 +176,10 @@ void bt_tunable::toggle(void)
good = good_bad();
if (good == TUNE_GOOD) {
- system("/usr/sbin/hciconfig hci0 up &> /dev/null &");
+ do_system("/usr/sbin/hciconfig hci0 up &> /dev/null &");
return;
}
- system("/usr/sbin/hciconfig hci0 down &> /dev/null");
+ do_system("/usr/sbin/hciconfig hci0 down &> /dev/null");
}
const char *bt_tunable::toggle_script(void)
diff --git a/src/tuning/cpufreq.cpp b/src/tuning/cpufreq.cpp
index df245ad..8a8d1c5 100644
--- a/src/tuning/cpufreq.cpp
+++ b/src/tuning/cpufreq.cpp
@@ -115,7 +115,7 @@ void cpufreq_tunable::toggle(void)
int good;
good = good_bad();
- system("/sbin/modprobe cpufreq_ondemand > /dev/null 2>&1");
+ do_system("/sbin/modprobe cpufreq_ondemand > /dev/null 2>&1");
if (good == TUNE_GOOD) {
dir = opendir("/sys/devices/system/cpu");
--
1.7.9.2
2:13 a.m.
New subject: [Powertop] [PATCH 3/3] process: Get rid of unused dbg_printf_pevent_info()
This function is not used, just remove it.
This fixes following warning:
CXX process/powertop-do_process.o
process/do_process.cpp:190: warning: ‘void dbg_printf_pevent_info(event_format*,
pevent_record*)’ defined but not used
Signed-off-by: Namhyung Kim <namhyung(a)gmail.com>
---
src/process/do_process.cpp | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/src/process/do_process.cpp b/src/process/do_process.cpp
index 6ac0fae..a07b3f3 100644
--- a/src/process/do_process.cpp
+++ b/src/process/do_process.cpp
@@ -187,19 +187,6 @@ int dont_blame_me(char *comm)
return 0;
}
-static void dbg_printf_pevent_info(struct event_format *event, struct pevent_record
*rec)
-{
- static struct trace_seq s;
-
- event->pevent->print_raw = 1;
- trace_seq_init(&s);
- pevent_event_info(&s, event, rec);
- trace_seq_putc(&s, '\n');
- trace_seq_terminate(&s);
- fprintf(stderr, "%.*s", s.len, s.buffer);
- trace_seq_destroy(&s);
-}
-
static char * get_pevent_field_str(void *trace, struct event_format *event, struct
format_field *field)
{
unsigned long long offset, len;
--
1.7.9.2
3:16 a.m.
New subject: [Powertop] [PATCH 3/3] process: Get rid of unused dbg_printf_pevent_info()
On (12/04/12 00:13), Namhyung Kim wrote:
This function is not used, just remove it.
This fixes following warning:
CXX process/powertop-do_process.o
process/do_process.cpp:190: warning: ‘void dbg_printf_pevent_info(event_format*,
pevent_record*)’ defined but not used
Signed-off-by: Namhyung Kim <namhyung(a)gmail.com>
---
src/process/do_process.cpp | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/src/process/do_process.cpp b/src/process/do_process.cpp
index 6ac0fae..a07b3f3 100644
--- a/src/process/do_process.cpp
+++ b/src/process/do_process.cpp
@@ -187,19 +187,6 @@ int dont_blame_me(char *comm)
return 0;
}
-static void dbg_printf_pevent_info(struct event_format *event, struct pevent_record
*rec)
-{
- static struct trace_seq s;
-
- event->pevent->print_raw = 1;
- trace_seq_init(&s);
- pevent_event_info(&s, event, rec);
- trace_seq_putc(&s, '\n');
- trace_seq_terminate(&s);
- fprintf(stderr, "%.*s", s.len, s.buffer);
- trace_seq_destroy(&s);
-}
-
static char * get_pevent_field_str(void *trace, struct event_format *event, struct
format_field *field)
{
unsigned long long offset, len;
this one looks good to me.
-ss
10:27 a.m.
New subject: [Powertop] [PATCH 3/3] process: Get rid of unused dbg_printf_pevent_info()
On Tue, 4 Dec 2012 00:13:20 +0900
Namhyung Kim <namhyung(a)gmail.com> wrote:
This function is not used, just remove it.
This fixes following warning:
CXX process/powertop-do_process.o
process/do_process.cpp:190: warning: ‘void
dbg_printf_pevent_info(event_format*, pevent_record*)’ defined but
not used
Signed-off-by: Namhyung Kim <namhyung(a)gmail.com>
---
src/process/do_process.cpp | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/src/process/do_process.cpp b/src/process/do_process.cpp
index 6ac0fae..a07b3f3 100644
--- a/src/process/do_process.cpp
+++ b/src/process/do_process.cpp
@@ -187,19 +187,6 @@ int dont_blame_me(char *comm)
return 0;
}
-static void dbg_printf_pevent_info(struct event_format *event,
struct pevent_record *rec) -{
- static struct trace_seq s;
-
- event->pevent->print_raw = 1;
- trace_seq_init(&s);
- pevent_event_info(&s, event, rec);
- trace_seq_putc(&s, '\n');
- trace_seq_terminate(&s);
- fprintf(stderr, "%.*s", s.len, s.buffer);
- trace_seq_destroy(&s);
-}
-
static char * get_pevent_field_str(void *trace, struct event_format
*event, struct format_field *field) {
unsigned long long offset, len;
Applied, thanks.
Kristen
3:14 a.m.
New subject: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()
On 12/3/2012 7:13 AM, Namhyung Kim wrote:
- write(fd, buffer, 64*1024);
+ if (write(fd, buffer, 64*1024))
+ /* make gcc silent */;
fdatasync(fd);
I'm sorry, but no.
This is waaaaaaaaaaay worse than leaving the compiler warning.
The compiler warning is real, and is a reminder that we need to improve the code
"just make it shut up" patches are a really bad idea, and this is the perfect
example of that.
3:18 a.m.
New subject: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()
Hi Arjan,
2012-12-03 (월), 08:14 -0800, Arjan van de Ven:
On 12/3/2012 7:13 AM, Namhyung Kim wrote:
> - write(fd, buffer, 64*1024);
> + if (write(fd, buffer, 64*1024))
> + /* make gcc silent */;
> fdatasync(fd);
I'm sorry, but no.
This is waaaaaaaaaaay worse than leaving the compiler warning.
The compiler warning is real, and is a reminder that we need to improve the code
"just make it shut up" patches are a really bad idea, and this is the perfect
example of that.
So how can we improve the code then?
--
Regards,
Namhyung Kim
3:25 a.m.
New subject: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()
-----Original Message-----
From: powertop-bounces(a)lists.01.org [mailto:powertop-bounces@lists.01.org] On Behalf Of
Namhyung Kim
Sent: Monday, December 03, 2012 8:18 AM
To: Arjan van de Ven
Cc: powertop(a)lists.01.org
Subject: Re: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()
Hi Arjan,
2012-12-03 (월), 08:14 -0800, Arjan van de Ven:
On 12/3/2012 7:13 AM, Namhyung Kim wrote:
> - write(fd, buffer, 64*1024);
> + if (write(fd, buffer, 64*1024))
> + /* make gcc silent */;
> fdatasync(fd);
I'm sorry, but no.
This is waaaaaaaaaaay worse than leaving the compiler warning.
The compiler warning is real, and is a reminder that we need to
improve the code "just make it shut up" patches are a really bad idea, and this
is the perfect example of that.
So how can we improve the code then?
The code needs to be refactored to handle the error.
-c
--
Regards,
Namhyung Kim
_______________________________________________
PowerTop mailing list
PowerTop(a)lists.01.org
https://lists.01.org/mailman/listinfo/powertop
3517
days inactive
3576
days old
7 comments
5 participants
participants (5)
-
Arjan van de Ven -
Ferron, Chris E -
Kristen Carlson Accardi -
Namhyung Kim -
Sergey Senozhatsky