| Message ID | 53DFD2B7.7080801@ramsay1.demon.co.uk (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
Compile-i386 is pretty much not maintained any more. On Tue, Aug 5, 2014 at 2:36 AM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote: > > - write(STDOUT_FILENO, s, strlen(s)); > + printf("%s", s); This change fix nothing. "printf" can return how many bytes it is written. In the case that "write" can fail, "printf" can fail as well. e.g. disk full. This patch did not check the return value from printf either. If we really care about return value of write, (we don't). We can do some thing like: if (write(...)< len) die("...."); That at least check the return value. Chris -- 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
On 27/09/14 03:13, Christopher Li wrote: > Compile-i386 is pretty much not maintained any more. > > On Tue, Aug 5, 2014 at 2:36 AM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote: >> >> - write(STDOUT_FILENO, s, strlen(s)); >> + printf("%s", s); > > This change fix nothing. "printf" can return how many bytes it is written. > In the case that "write" can fail, "printf" can fail as well. e.g. disk full. > This patch did not check the return value from printf either. I was only really fixing the compiler warning (in an side-ways fashion). (although I think it is an improvement). If this is no longer being maintained, perhaps a patch to remove it would be better? 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
On Sat, Sep 27, 2014 at 9:00 PM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote: >>> - write(STDOUT_FILENO, s, strlen(s)); >>> + printf("%s", s); >> > > I was only really fixing the compiler warning (in an side-ways fashion). > (although I think it is an improvement). I would just check the return value and "die" if there is an error to silence the error. Chris -- 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
On 30/09/14 03:34, Christopher Li wrote: > On Sat, Sep 27, 2014 at 9:00 PM, Ramsay Jones > <ramsay@ramsay1.demon.co.uk> wrote: >>>> - write(STDOUT_FILENO, s, strlen(s)); >>>> + printf("%s", s); >>> >> >> I was only really fixing the compiler warning (in an side-ways fashion). >> (although I think it is an improvement). > > I would just check the return value and "die" if there is an error to silence > the error. Hmmm, I can change the patch to do that if you prefer. Having said that, given that this is not currently maintained, what do you think about a patch to remove it? [Are you hoping someone will pick it up again soon?] Let me know which patch you would like to see. 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 --git a/compile-i386.c b/compile-i386.c index 88169ec..f89b6ca 100644 --- a/compile-i386.c +++ b/compile-i386.c @@ -732,7 +732,7 @@ static void emit_insn_atom(struct function *f, struct atom *atom) atom->insn, comment[0] ? "\t\t" : "", comment); - write(STDOUT_FILENO, s, strlen(s)); + printf("%s", s); } static void emit_atom_list(struct function *f) @@ -742,9 +742,7 @@ 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 */ + printf("%.*s", (int)atom->text_len, atom->text); break; } case ATOM_INSN: @@ -886,7 +884,6 @@ static void emit_func_post(struct symbol *sym) insn("ret", NULL, NULL, NULL); /* output everything to stdout */ - fflush(stdout); /* paranoia; needed? */ emit_atom_list(f); /* function footer */
Some versions of gcc (e.g. v4.8.2) complain about ignoring the return value of a call to 'write(2)' on line 735, since it is marked with the warn_unused_result attribute. (see also lines 745-747). Note that this code is mixing calls to 'write' on STDOUT_FILENO with calls to stdio functions on the stdout file handle. So, in order to suppress the compiler warnings, we replace both calls to 'write' with equivalent calls to stdio functions. Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> --- Hi Chris, Since there are no tests for the compile program, I ran the old version (from commit 748b856) and this version over all of the .c files in the sparse directory, saving the output to file and diff-ing the result. Unfortunately, compile segfaults on most files and the diff shows that this patch leads to a change in behaviour, caused by the lack of I/O buffer flushing prior to the segfault (all of the diffs showed missing text at the end of each new output file). Just to confirm, I added back the 'fflush(stdout); ...' line that I removed in the third hunk below _and_ duplicated that line and placed it after the call to 'emit_atom_list(f);'. This version of compile produced (almost) exactly the same output as the original. The reason for the almost is that three files showed a non-empty diff which seemed to indicate (a possible bug?) that a label name was being generated from a memory address. The actual addresses/label name generated in each file was different, but they were used consistently in each file. For example, the complete diff for sort.c is: --- sort.c.out-old 2014-08-04 17:32:32.399744973 +0100 +++ sort.c.out-new 2014-08-04 17:37:04.189092702 +0100 @@ -742,7 +742,7 @@ addl $744, %esp ret .size merge_block_seqs, .-merge_block_seqs -.L0xb740550c: +.L0xb748068c: .globl sort_list .type sort_list, @function sort_list: @@ -837,7 +837,7 @@ movl %eax, 68(%esp) # .... end copy jmp .L72 .L87: # end if - jmp .LS0xb740550c + jmp .LS0xb748068c .L86: # end if movl 44(%esp), %eax # begin copy .. movl %eax, 72(%esp) # .... end copy ATB, Ramsay Jones compile-i386.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)