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