[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1551917 - /subversion/trunk/subversion/svnserve/logger.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Wed, 18 Dec 2013 22:22:26 +0100

> -----Original Message-----
> From: stsp_at_apache.org [mailto:stsp_at_apache.org]
> Sent: woensdag 18 december 2013 12:29
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1551917 -
> /subversion/trunk/subversion/svnserve/logger.c
>
> Author: stsp
> Date: Wed Dec 18 11:29:29 2013
> New Revision: 1551917
>
> URL: http://svn.apache.org/r1551917
> Log:
> * subversion/svnserve/logger.c
> (logger__log_error): Replace strcpy()/strlen() calls with a single call
> to apr_snprintf() which performs bounds checking and calculates the
> length for us. No functional change.
>
> Modified:
> subversion/trunk/subversion/svnserve/logger.c
>
> Modified: subversion/trunk/subversion/svnserve/logger.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/logge
> r.c?rev=1551917&r1=1551916&r2=1551917&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/svnserve/logger.c (original)
> +++ subversion/trunk/subversion/svnserve/logger.c Wed Dec 18 11:29:29
> 2013
> @@ -149,8 +149,8 @@ logger__log_error(logger_t *logger,
> if (len > sizeof(errstr) - sizeof(APR_EOL_STR)) {
> len = sizeof(errstr) - sizeof(APR_EOL_STR);
> }
> - strcpy(errstr + len, APR_EOL_STR);
> - len += strlen(APR_EOL_STR);
> + len += apr_snprintf(errstr + len, sizeof(APR_EOL_STR),
> + "%s", APR_EOL_STR);
> svn_error_clear(svn_stream_write(logger->stream, errstr, &len));

Can you explain why you changed this?

This changes one bit of dynamic calculation to another bit of dynamic calculation while this string is completely constant. You now use the compiler to optimize away a strlen, but apr_snprintf() has a lot more overhead in other places (variable arguments, parsing the format string, etc.).

In this case I'm really wondering why we change working code...?

        Bert
Received on 2013-12-18 22:23:10 CET

This is an archived mail posted to the Subversion Dev mailing list.