[RFC,3/7] error reporting: Use error_report_errno in obvious places

Commit Message

Eric Blake April 26, 2018, 6:07 p.m. UTC

On 04/26/2018 12:43 PM, Ian Jackson wrote:
> Eric Blake writes ("Re: [RFC PATCH 3/7] error reporting: Use error_report_errno in obvious places"):
>> Misses a lot of two-line instances, such as:
>> $ git grep -A1 error_report | grep -C1 strerror.errno
>> ...
> ...
>> If we're going to clean up these instances, we might as well look harder
>> for them.  Can Coccinelle be coaxed into helping us (at least for
>> identifying callsites, even if I can't figure out how to make it
>> slice-and-dice format strings)?
> 
> I have never used Coccinelle, so I don't know.

It was quite easy to detect spots that would benefit from this pattern;
I'm less certain about getting Coccinelle to rewrite them, but at least
knowing where they are makes it easier to ensure you aren't missing
obvious candidates:

$ cat error_report.cocci
@@
expression E;
@@
* error_report(..., strerror(E))

$ spatch --sp-file error_report.cocci \
  --macro-file scripts/cocci-macro-file.h --dir . 2>/dev/null \
  grep -c error_report
149

Between patch 2 and 5, I thus see 149 instances of a call to strerror()
embedded as the last parameter to error_report(), which is more than you
found.

As an example, the tail of the Coccinelle output shows the one in
block/file-posix.c that I demonstrated that you missed, plus the one in
util/osdep.c that your simpler perl script caught:

...

Patch

--- ./block/file-posix.c
+++ /tmp/nothing/block/file-posix.c
@@ -1762,8 +1762,6 @@  static int raw_regular_truncate(int fd,
 out:
     if (result < 0) {
         if (ftruncate(fd, current_length) < 0) {
-            error_report("Failed to restore old file length: %s",
-                         strerror(errno));
         }
     }

diff -u -p ./util/osdep.c /tmp/nothing/util/osdep.c
--- ./util/osdep.c
+++ /tmp/nothing/util/osdep.c
@@ -89,7 +89,6 @@  static int qemu_mprotect__osdep(void *ad
     return 0;
 #else
     if (mprotect(addr, size, prot)) {
-        error_report("%s: mprotect failed: %s", __func__, strerror(errno));
         return -1;
     }
     return 0;