You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Reser <be...@reser.org> on 2012/10/15 16:18:19 UTC

Re: svn commit: r1398252 - /subversion/trunk/subversion/libsvn_subr/error.c

On Mon, Oct 15, 2012 at 7:30 AM,  <iv...@apache.org> wrote:
> Author: ivan
> Date: Mon Oct 15 11:30:08 2012
> New Revision: 1398252
>
> URL: http://svn.apache.org/viewvc?rev=1398252&view=rev
> Log:
> Make code a little bit more clear and faster.
>
> * subversion/libsvn_subr/error.c
>   (svn_error_wrap_apr): Use apr_pstrcat() instead of apr_psprintf().
>
> Modified:
>     subversion/trunk/subversion/libsvn_subr/error.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/error.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/error.c?rev=1398252&r1=1398251&r2=1398252&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/error.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/error.c Mon Oct 15 11:30:08 2012
> @@ -196,9 +196,14 @@ svn_error_wrap_apr(apr_status_t status,
>        va_start(ap, fmt);
>        msg = apr_pvsprintf(err->pool, fmt, ap);
>        va_end(ap);
> -      err->message = apr_psprintf(err->pool, "%s%s%s", msg,
> -                                  (msg_apr) ? ": " : "",
> -                                  (msg_apr) ? msg_apr : "");
> +      if (msg_apr)
> +        {
> +          err->message = apr_pstrcat(err->pool, msg, ": ", msg_apr, NULL);
> +        }
> +      else
> +        {
> +          err->message = msg;
> +        }
>      }
>
>    return err;
>

That's not functionally equivalent.  If msg_apr is NULL then you'll
get a trailing ": " when you couldn't have otherwise.

This would be equivalent:
 err->message = apr_pstrcat(err->pool, msg, (msg_apr) ? ": " : "",
msg_apr, NULL);

Re: svn commit: r1398252 - /subversion/trunk/subversion/libsvn_subr/error.c

Posted by Ben Reser <be...@reser.org>.
On Mon, Oct 15, 2012 at 10:18 AM, Ben Reser <be...@reser.org> wrote:
> On Mon, Oct 15, 2012 at 7:30 AM,  <iv...@apache.org> wrote:
>> Author: ivan
>> Date: Mon Oct 15 11:30:08 2012
>> New Revision: 1398252
>>
>> URL: http://svn.apache.org/viewvc?rev=1398252&view=rev
>> Log:
>> Make code a little bit more clear and faster.
>>
>> * subversion/libsvn_subr/error.c
>>   (svn_error_wrap_apr): Use apr_pstrcat() instead of apr_psprintf().
>>
>> Modified:
>>     subversion/trunk/subversion/libsvn_subr/error.c
>>
>> Modified: subversion/trunk/subversion/libsvn_subr/error.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/error.c?rev=1398252&r1=1398251&r2=1398252&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_subr/error.c (original)
>> +++ subversion/trunk/subversion/libsvn_subr/error.c Mon Oct 15 11:30:08 2012
>> @@ -196,9 +196,14 @@ svn_error_wrap_apr(apr_status_t status,
>>        va_start(ap, fmt);
>>        msg = apr_pvsprintf(err->pool, fmt, ap);
>>        va_end(ap);
>> -      err->message = apr_psprintf(err->pool, "%s%s%s", msg,
>> -                                  (msg_apr) ? ": " : "",
>> -                                  (msg_apr) ? msg_apr : "");
>> +      if (msg_apr)
>> +        {
>> +          err->message = apr_pstrcat(err->pool, msg, ": ", msg_apr, NULL);
>> +        }
>> +      else
>> +        {
>> +          err->message = msg;
>> +        }
>>      }
>>
>>    return err;
>>
>
> That's not functionally equivalent.  If msg_apr is NULL then you'll
> get a trailing ": " when you couldn't have otherwise.
>
> This would be equivalent:
>  err->message = apr_pstrcat(err->pool, msg, (msg_apr) ? ": " : "",
> msg_apr, NULL);

Nevermind, msg_apr can't be NULL there.