You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by iv...@apache.org on 2010/01/26 11:12:05 UTC

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

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));
     }
 
   return parser_status;



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

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jan 26, 2010 at 3:23 PM, Greg Stein <gs...@gmail.com> wrote:
> On Tue, Jan 26, 2010 at 07:13, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On Tue, Jan 26, 2010 at 3:01 PM, Greg Stein <gs...@gmail.com> wrote:
>>> On Tue, Jan 26, 2010 at 06:59, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>> On Tue, Jan 26, 2010 at 2:41 PM, Greg Stein <gs...@gmail.com> wrote:
>>>>> On Tue, Jan 26, 2010 at 05:12,  <iv...@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.
>
> That strange wrapping is from before we voted for "no whitespace
> before paren" some years ago. Those cases will (eventually) be
> corrected. (and please feel free to do so yourself)
Yes, I remember that vote :)

>
> The following script is handy: trunk/tools/dev/find-bad-style.py
>
Ok, I fixed formatting in r903700.


-- 
Ivan Zhakov
VisualSVN Team

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

Posted by Greg Stein <gs...@gmail.com>.
On Tue, Jan 26, 2010 at 07:13, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Tue, Jan 26, 2010 at 3:01 PM, Greg Stein <gs...@gmail.com> wrote:
>> On Tue, Jan 26, 2010 at 06:59, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>> On Tue, Jan 26, 2010 at 2:41 PM, Greg Stein <gs...@gmail.com> wrote:
>>>> On Tue, Jan 26, 2010 at 05:12,  <iv...@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.

That strange wrapping is from before we voted for "no whitespace
before paren" some years ago. Those cases will (eventually) be
corrected. (and please feel free to do so yourself)

The following script is handy: trunk/tools/dev/find-bad-style.py

Cheers,
-g

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

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jan 26, 2010 at 3:01 PM, Greg Stein <gs...@gmail.com> wrote:
> On Tue, Jan 26, 2010 at 06:59, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On Tue, Jan 26, 2010 at 2:41 PM, Greg Stein <gs...@gmail.com> wrote:
>>> On Tue, Jan 26, 2010 at 05:12,  <iv...@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

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

Posted by Greg Stein <gs...@gmail.com>.
On Tue, Jan 26, 2010 at 06:59, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Tue, Jan 26, 2010 at 2:41 PM, Greg Stein <gs...@gmail.com> wrote:
>> On Tue, Jan 26, 2010 at 05:12,  <iv...@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.

Cheers,
-g

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

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jan 26, 2010 at 2:41 PM, Greg Stein <gs...@gmail.com> wrote:
> On Tue, Jan 26, 2010 at 05:12,  <iv...@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.


-- 
Ivan Zhakov
VisualSVN Team

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

Posted by Greg Stein <gs...@gmail.com>.
On Tue, Jan 26, 2010 at 05:12,  <iv...@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.

Cheers,
-g