Re: svn commit: r903153 - /subversion/trunk/subversion/libsvn_ra_neon/util.c

On Tue, Jan 26, 2010 at 3:01 PM, Greg Stein <gstein_at_gmail.com> wrote:
> On Tue, Jan 26, 2010 at 06:59, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>> On Tue, Jan 26, 2010 at 2:41 PM, Greg Stein <gstein_at_gmail.com> wrote:
>>> On Tue, Jan 26, 2010 at 05:12,  <ivan_at_apache.org> wrote:
>>>> Author: ivan
>>>> Date: Tue Jan 26 10:12:05 2010
>>>> New Revision: 903153
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=903153&view=rev
>>>> Log:
>>>> Follow up to r902836.
>>>>
>>>> * subversion/libsvn_ra_neon/util.c
>>>>  (wrapper_reader_cb): Use SVN_RA_NEON__REQ_ERR macro instead of
>>>>   assigning req->err directly to prevent potential error leakage.
>>>>
>>>> Modified:
>>>>    subversion/trunk/subversion/libsvn_ra_neon/util.c
>>>>
>>>> Modified: subversion/trunk/subversion/libsvn_ra_neon/util.c
>>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_neon/util.c?rev=903153&r1=903152&r2=903153&view=diff
>>>> ==============================================================================
>>>> --- subversion/trunk/subversion/libsvn_ra_neon/util.c (original)
>>>> +++ subversion/trunk/subversion/libsvn_ra_neon/util.c Tue Jan 26 10:12:05 2010
>>>> @@ -1123,8 +1123,9 @@
>>>>   if (parser_status)
>>>>     {
>>>>       /* Pass XML parser error. */
>>>> -      pwb->req->err = check_parse_error(pwb->req->method, pwb->parser,
>>>> -                                        pwb->req->url);
>>>> +      SVN_RA_NEON__REQ_ERR
>>>> +        (pwb->req, check_parse_error(pwb->req->method, pwb->parser,
>>>> +                                     pwb->req->url));
>>>
>>> There should be no white space before the function-call open paren.
>>>
>> Hi Greg,
>>
>> Sorry I didn't understand what white space do you mean. There is no
>> white spaces after SVN_RA_NEON__REQ_ERR macro and arguments are
>> wrapped on next line.
>
> The whitespace between the macro name and the paren. It should be:
>
> SVN_RA_NEON__REQ_ERR(...
>
> The arguments can then be wrapped to the next line, but the
> parentheses should never be wrapped in a way that introduces
> whitespace between the function/macro name and the paren.
>
That's a bit strange since we're wrapping parent on next line in many
places and I've just copied formatting from other place. For example
it occurs 391 times in call to svn_error_createf().
[[
        return svn_error_createf
          (SVN_ERR_CLIENT_IS_BINARY_FILE, 0,
           _("Cannot calculate blame information for binary file '%s'"),
           svn_dirent_local_style(target, pool));
]]

Just to be clear it's not a problem for me to change formatting, but
I'm worry about consistency.

-- 
Ivan Zhakov
VisualSVN Team