diff mbox

compile-i386.c: don't ignore return value of write(2)

Message ID 543C5A43.9@ramsay1.demon.co.uk (mailing list archive)
State Mainlined, archived
Headers show

Commit Message

Ramsay Jones Oct. 13, 2014, 11:03 p.m. UTC
Some versions of gcc (e.g. v4.8.2) complain about ignoring the return
value of a call to the write(2) system call, since the system header
files have marked its declaration with the warn_unused_result attribute.

In order to suppress the compiler warning, check the return value from
'write' and, if it indicates an error (a negative return value), exit
the process using 'die' to display an error message. Replace a second
call to 'write', which does not provoke a compiler warning, with similar
code for consistency.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Chris,

This is an (almost) minimal patch to suppress the compiler warning.
You could drop the second hunk (and edit the commit message) if you
prefer an absolutely minimal patch. :-D

ATB,
Ramsay Jones

 compile-i386.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Josh Triplett Oct. 14, 2014, 3:14 a.m. UTC | #1
On Tue, Oct 14, 2014 at 12:03:31AM +0100, Ramsay Jones wrote:
> 
> Some versions of gcc (e.g. v4.8.2) complain about ignoring the return
> value of a call to the write(2) system call, since the system header
> files have marked its declaration with the warn_unused_result attribute.
> 
> In order to suppress the compiler warning, check the return value from
> 'write' and, if it indicates an error (a negative return value), exit
> the process using 'die' to display an error message. Replace a second
> call to 'write', which does not provoke a compiler warning, with similar
> code for consistency.
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>

That isn't the only way write() can fail; it can also do a partial
write, in which case you need to loop and write the rest.  You might
consider adding an xwrite() function which includes that logic.
(Alternatively, you could use standard C IO, which doesn't have that
problem.)

> 
> Hi Chris,
> 
> This is an (almost) minimal patch to suppress the compiler warning.
> You could drop the second hunk (and edit the commit message) if you
> prefer an absolutely minimal patch. :-D
> 
> ATB,
> Ramsay Jones
> 
>  compile-i386.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/compile-i386.c b/compile-i386.c
> index 88169ec..44b72ec 100644
> --- a/compile-i386.c
> +++ b/compile-i386.c
> @@ -732,7 +732,8 @@ static void emit_insn_atom(struct function *f, struct atom *atom)
>  			atom->insn,
>  			comment[0] ? "\t\t" : "", comment);
>  
> -	write(STDOUT_FILENO, s, strlen(s));
> +	if (write(STDOUT_FILENO, s, strlen(s)) < 0)
> +		die("can't write to stdout");
>  }
>  
>  static void emit_atom_list(struct function *f)
> @@ -742,9 +743,8 @@ static void emit_atom_list(struct function *f)
>  	FOR_EACH_PTR(f->atom_list, atom) {
>  		switch (atom->type) {
>  		case ATOM_TEXT: {
> -			ssize_t rc = write(STDOUT_FILENO, atom->text,
> -					   atom->text_len);
> -			(void) rc;	/* FIXME */
> +			if (write(STDOUT_FILENO, atom->text, atom->text_len) < 0)
> +				die("can't write to stdout");
>  			break;
>  		}
>  		case ATOM_INSN:
> -- 
> 2.1.0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ramsay Jones Oct. 14, 2014, 10:13 a.m. UTC | #2
On 14/10/14 04:14, Josh Triplett wrote:
> On Tue, Oct 14, 2014 at 12:03:31AM +0100, Ramsay Jones wrote:
>>
>> Some versions of gcc (e.g. v4.8.2) complain about ignoring the return
>> value of a call to the write(2) system call, since the system header
>> files have marked its declaration with the warn_unused_result attribute.
>>
>> In order to suppress the compiler warning, check the return value from
>> 'write' and, if it indicates an error (a negative return value), exit
>> the process using 'die' to display an error message. Replace a second
>> call to 'write', which does not provoke a compiler warning, with similar
>> code for consistency.
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> 
> That isn't the only way write() can fail; it can also do a partial
> write, in which case you need to loop and write the rest.  You might
> consider adding an xwrite() function which includes that logic.

I did consider doing exactly that, but I wanted a _minimal_ fix for a
program that is basically unmaintained. However, if Chris would like
such a patch, I will happily provide one! ;-)

> (Alternatively, you could use standard C IO, which doesn't have that
> problem.)

See the very first version of this patch. :-D (I still slightly prefer
the first patch, very closely followed by the second version; this is
the third attempt).

ATB,
Ramsay Jones



--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/compile-i386.c b/compile-i386.c
index 88169ec..44b72ec 100644
--- a/compile-i386.c
+++ b/compile-i386.c
@@ -732,7 +732,8 @@  static void emit_insn_atom(struct function *f, struct atom *atom)
 			atom->insn,
 			comment[0] ? "\t\t" : "", comment);
 
-	write(STDOUT_FILENO, s, strlen(s));
+	if (write(STDOUT_FILENO, s, strlen(s)) < 0)
+		die("can't write to stdout");
 }
 
 static void emit_atom_list(struct function *f)
@@ -742,9 +743,8 @@  static void emit_atom_list(struct function *f)
 	FOR_EACH_PTR(f->atom_list, atom) {
 		switch (atom->type) {
 		case ATOM_TEXT: {
-			ssize_t rc = write(STDOUT_FILENO, atom->text,
-					   atom->text_len);
-			(void) rc;	/* FIXME */
+			if (write(STDOUT_FILENO, atom->text, atom->text_len) < 0)
+				die("can't write to stdout");
 			break;
 		}
 		case ATOM_INSN: