[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 6/9] target-i386: Handle I/O breakpoints
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v2 6/9] target-i386: Handle I/O breakpoints |
Date: |
Tue, 20 Oct 2015 14:04:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 19/10/2015 19:57, Eduardo Habkost wrote:
> 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.
Because Emacs indents this:
> + return hw_breakpoint_enabled(env->dr[7], index)
> + ? HF_IOBPT_MASK : 0;
with the ? under the second "r" of "return", while it indents this as
written:
> - return (hw_breakpoint_enabled(env->dr[7], index)
> - ? HF_IOBPT_MASK : 0);
Another random example:
static bool sdl_check_format(DisplayChangeListener *dcl,
pixman_format_code_t format)
{
/*
* We let SDL convert for us a few more formats than,
* the native ones. Thes are the ones I have tested.
*/
return (format == PIXMAN_x8r8g8b8 ||
format == PIXMAN_b8g8r8x8 ||
format == PIXMAN_x1r5g5b5 ||
format == PIXMAN_r5g6b5);
}
There's no unanimity though, so your v3 is okay too.
Paolo
- [Qemu-devel] [PATCH v2 1/9] target-i386: Introduce cpu_x86_update_dr7, (continued)
- [Qemu-devel] [PATCH v2 1/9] target-i386: Introduce cpu_x86_update_dr7, Eduardo Habkost, 2015/10/16
- [Qemu-devel] [PATCH v2 2/9] target-i386: Re-introduce optimal breakpoint removal, Eduardo Habkost, 2015/10/16
- [Qemu-devel] [PATCH v2 3/9] target-i386: Ensure bit 10 on DR7 is never cleared, Eduardo Habkost, 2015/10/16
- [Qemu-devel] [PATCH v2 4/9] target-i386: Move hw_*breakpoint_* functions, Eduardo Habkost, 2015/10/16
- [Qemu-devel] [PATCH v2 5/9] target-i386: Optimize setting dr[0-3], Eduardo Habkost, 2015/10/16
- [Qemu-devel] [PATCH v2 6/9] target-i386: Handle I/O breakpoints, Eduardo Habkost, 2015/10/16
[Qemu-devel] [PATCH v2 7/9] target-i386: Check CR4[DE] for processing DR4/DR5, Eduardo Habkost, 2015/10/16
[Qemu-devel] [PATCH v2 8/9] target-i386: Ensure always-1 bits on DR6 can't be cleared, Eduardo Habkost, 2015/10/16
[Qemu-devel] [PATCH v2 9/9] target-i386: Add DE to TCG_FEATURES, Eduardo Habkost, 2015/10/16