Re: svn commit: r1004286 - in /subversion/trunk: ./ subversion/libsvn_subr/io.c

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
Received on 2011-01-11 17:58:05 CET