You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2005/05/27 16:46:01 UTC

Re: svn commit: r14859 - trunk/subversion/libsvn_subr

fitz@tigris.org writes:
> --- trunk/subversion/libsvn_subr/io.c	(original)
> +++ trunk/subversion/libsvn_subr/io.c	Fri May 27 10:34:50 2005
> @@ -999,35 +999,45 @@
>    return SVN_NO_ERROR;
>  }
>  
> -/* Create a temp file so that we can use the temp file's mask when
> -   setting PATH (and any other file for the life of the process) to
> -   read-write (on Unix).  */
> +/* Set PERMS to the default file permissions for PATH based on the
> +   permissions of PATH and a newly created file.  Make temporary
> +   allocations in POOL.  */
>  static svn_error_t *
>  get_default_file_perms (const char *path, apr_fileperms_t *perms,
>                          apr_pool_t *pool)

I don't understand the new doc string (or rather, I wouldn't
understand it if I didn't have the old doc string for context).

Is it saying that a new file gets created, and if so, how is that new
file related to PATH?  What is the relationship of these things to
PERMS, exactly?

It's a pity there isn't some way to do this without creating a new
file temporarily.  The name 'get_default_file_perms' implies
read-onlyness in my mind.

>  {
>    apr_status_t status;
> -  apr_finfo_t finfo;
> +  apr_finfo_t tmp_finfo, finfo;
>    apr_file_t *fd;
>    const char *tmp_path;
>    const char *apr_path;
>  
> -  SVN_ERR (svn_path_cstring_from_utf8 (&apr_path, path, pool));
> +  /* Get the perms for a newly created file to find out what write
> +   * bits should be set. */
>    SVN_ERR (svn_io_open_unique_file (&fd, &tmp_path, path, 
>                                      "tmp", TRUE, pool));
> -  status = apr_stat (&finfo, tmp_path, APR_FINFO_PROT, pool);
> +  status = apr_stat (&tmp_finfo, tmp_path, APR_FINFO_PROT, pool);
>    if (status)
>      return svn_error_wrap_apr (status, _("Can't get default file perms "
>                                           "for file at '%s' (file stat error)"),

Not part of your change, but I'd think we want ".tmp" there instead of
just "tmp".

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org