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
