Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

[Eduardo Habkost]

On Mon, Jun 13, 2016 at 10:01:16AM -0600, Eric Blake wrote:
> On 06/13/2016 09:52 AM, Eduardo Habkost wrote:
[...]
> > 
> > See, e.g.:
> > 
> > void qmp_guest_suspend_disk(Error **errp)
> > {
> >     Error *local_err = NULL;
> >     GuestSuspendMode *mode = g_new(GuestSuspendMode, 1);
> > 
> >     *mode = GUEST_SUSPEND_MODE_DISK;
> >     check_suspend_mode(*mode, &local_err);
> >     acquire_privilege(SE_SHUTDOWN_NAME, &local_err);
> >     execute_async(do_suspend, mode, &local_err);
> 
> That usage is a bug.  A Coccinelle script to root out such buggy
> instances would be nice.

I've tried to write one, but got lots of false positives due to
error-checking using the function return value, not local_err.
For reference, this is the script I have used:

@@
typedef Error;
identifier F1 !~ "hmp_handle_error|error_free_or_abort";
identifier F2 !~ "hmp_handle_error|error_free_or_abort";
idexpression Error *ERR;
@@
-F1(..., &ERR)
+FIXME_incorrect_error_usage1()
... when != ERR
-F2(..., &ERR)
+FIXME_incorrect_error_usage2()

@@
identifier F1 !~ "hmp_handle_error|error_free_or_abort";
identifier F2 !~ "hmp_handle_error|error_free_or_abort";
idexpression Error **ERRP;
@@
-F1(..., ERRP)
+FIXME_incorrect_error_usage1()
 ... when != ERRP
-F2(..., ERRP)
+FIXME_incorrect_error_usage2()

-- 
Eduardo