rhuijben_at_tigris.org writes:
> Author: rhuijben
> Date: Fri Sep 19 01:41:23 2008
> New Revision: 33178
>
> Log:
> Avoid calling apr_status with APR_FINFO_PROT | APR_FINFO_OWNER on Windows, as
> this call is 'incredibly expensive, but accurate' and the result is ignored
> anyway.
>
> * subversion/libsvn_subr/io.c
> (dir_make): Skip trying to set the unavailable sgid flag for WIN32.
> This avoids an 'incredibly expensive' ACL retrieval in apr.
> * subversion/libsvn_client/export.c
> (copy_versioned_files): Call svn_io_dir_make with APR_OS_DEFAULT permissions
> to avoid an 'incredibly expensive' ACL retrieval in apr.
Please put comments describing the code in the code and not in the log
message. The log message should describe the change:
"Avoid unnecessary frobnication on Windows"
the comments in the code should describe the code:
/* No need to frobnicate on Windows as it happens automatically
when the sun goes down. */
>
> Modified:
> trunk/subversion/libsvn_client/export.c
> trunk/subversion/libsvn_subr/io.c
>
> Modified: trunk/subversion/libsvn_client/export.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/export.c?pathrev=33178&r1=33177&r2=33178
> ==============================================================================
> --- trunk/subversion/libsvn_client/export.c Thu Sep 18 23:00:28 2008 (r33177)
> +++ trunk/subversion/libsvn_client/export.c Fri Sep 19 01:41:23 2008 (r33178)
> @@ -249,8 +249,12 @@ copy_versioned_files(const char *from,
> /* Try to make the new directory. If this fails because the
> directory already exists, check our FORCE flag to see if we
> care. */
> +#ifndef WIN32
> SVN_ERR(svn_io_stat(&finfo, from, APR_FINFO_PROT, pool));
> err = svn_io_dir_make(to, finfo.protection, pool);
> +#else
> + err = svn_io_dir_make(to, APR_OS_DEFAULT, pool);
> +#endif
How is a non-Windows programmer supposed to understand why the Windows
code is different? Please add a comment.
> if (err)
> {
> if (! APR_STATUS_IS_EEXIST(err->apr_err))
>
> Modified: trunk/subversion/libsvn_subr/io.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/io.c?pathrev=33178&r1=33177&r2=33178
> ==============================================================================
> --- trunk/subversion/libsvn_subr/io.c Thu Sep 18 23:00:28 2008 (r33177)
> +++ trunk/subversion/libsvn_subr/io.c Fri Sep 19 01:41:23 2008 (r33178)
> @@ -2800,6 +2800,7 @@ dir_make(const char *path, apr_fileperms
> }
> #endif
>
> +#ifndef WIN32
> if (sgid)
> {
> apr_finfo_t finfo;
> @@ -2811,6 +2812,7 @@ dir_make(const char *path, apr_fileperms
> if (!status)
> apr_file_perms_set(path_apr, finfo.protection | APR_GSETID);
> }
> +#endif
Same question as above.
It seems odd that it is expensive to get permissions if the returned
values are useless. Is this an apr bug? Could we change apr so that
the conditional code in Subversion is unnecessary?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-19 14:53:43 CEST