On Tue, 2011-01-11, Daniel Shahaf wrote:
> I believe this is semantically correct.
> 
> Ramkumar, Julian: is the struct still needed?  IIRC the reasons for
> originally introducing (namely the 'volatile' qualifier) it have since
> disappeared.

It's not strictly needed but it's a good way to structure the code
anyway.

Another question: what is this bit of code for?

> + if (*default_perms != 0)
> +    /* Nothing to do */
> +    return SVN_NO_ERROR;

I thought the function was guaranteed to be called only once, in which
case this would be redundant.

(I stared at this because it makes an assumption that 0 is not a valid
or at least not a likely permissions result.  It would still work
correctly if the result was 0, as this is just a speed optimization.)

In fact, this initialization is redundant too, isn't it ...

>  static svn_error_t *
>  get_default_file_perms(apr_fileperms_t *perms, apr_pool_t
> *scratch_pool)
>  {
> -  /* the default permissions as read from the temp folder */
>     static apr_fileperms_t default_perms = 0;

... here?

- Julian


Reply via email to