Re: [Qemu-devel] [PATCH v2 6/9] target-i386: Handle I/O breakpoints

[Eduardo Habkost]

On Mon, Oct 19, 2015 at 07:46:51AM -1000, Richard Henderson wrote:
> On 10/19/2015 07:30 AM, Eduardo Habkost wrote:
> >>>+        /* Notice when we should enable calls to bpt_io.  */
> >>>+        return (hw_breakpoint_enabled(env->dr[7], index)
> >>>+                ? HF_IOBPT_MASK : 0);
> >checkpatch.pl error:
> >
> >   ERROR: return is not a function, parentheses are not required
> >   #57: FILE: target-i386/bpt_helper.c:69:
> >   +        return (hw_breakpoint_enabled(env->dr[7], index)
> >
> >   total: 1 errors, 0 warnings, 242 lines checked
> >
> >I will fix it in v3.
> 
> In this case checkpatch is wrong, imo.  The parenthesis are not there to
> "make return a function", but to make the multi-line expression indent
> properly.

I understand if one thinks the expression looks better with the parenthesis,
but I fail to see why they are needed to indent the expression properly.

For reference, this is the change I have made in v3:

    diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
    index 0fbdc03..dac1b1a 100644
    --- a/target-i386/bpt_helper.c
    +++ b/target-i386/bpt_helper.c
    @@ -66,8 +66,8 @@ static int hw_breakpoint_insert(CPUX86State *env, int 
index)
    
         case DR7_TYPE_IO_RW:
             /* Notice when we should enable calls to bpt_io.  */
    -        return (hw_breakpoint_enabled(env->dr[7], index)
    -                ? HF_IOBPT_MASK : 0);
    +        return hw_breakpoint_enabled(env->dr[7], index)
    +               ? HF_IOBPT_MASK : 0;
    
         case DR7_TYPE_DATA_WR:
             if (hw_breakpoint_enabled(dr7, index)) {

-- 
Eduardo