Re: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()

Re: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()

[Namhyung Kim]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

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

Re: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()

[Ferron, Chris E]

[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]

-----Original Message-----
From: powertop-bounces(a)lists.01.org [mailto:powertop-bounces(a)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

Re: [Powertop] [PATCH 1/3] Suppress compiler warnings on write()

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

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.

[Powertop] [PATCH 1/3] Suppress compiler warnings on write()

[Namhyung Kim]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]

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