RE: [PATCH] Fix for renaming errors on Samba working copy

On 8 Oct 2008 at 10:43, Bert Huijben wrote:

> > -----Original Message-----
> > From: Alan Wood [mailto:alan.wood_at_clear.net.nz]
> > Sent: woensdag 8 oktober 2008 9:41
> > To: dev_at_subversion.tigris.org
> > Subject: RE: [PATCH] Fix for renaming errors on Samba working cop
> >
> > On 7 Oct 2008 at 12:15, Bert Huijben wrote:
> >
> > >
> > > I see the problem you experience and I think we can resolve it, but
> > your
> > > patch might fail before the original retry loop if the rename fails
> > for
> > > another reason.
> > >
> > > If the file is locked the SetAtttributes might fail. In this case
> > > SVN_ERR(..) will return the error before retrying the rename itself.
> > >
> > Replacing the retry loop with this should achieve the correct flow, but
> > I haven't
> > seen many coding standards that would approve :-)
> >
> > WIN32_RETRY_LOOP(status,
> > (APR_TO_OS_ERROR(status) == ERROR_ACCESS_DENIED &&
> > svn_io_set_file_read_write(from_path, TRUE, pool),
> > apr_file_rename(from_path_apr, to_path_apr, pool)));
>
> The retry loop checks the exact error. This code checks a boolean against
> several apr_status_t values ;)

Actually the retry macro only sees the return form the apr_file_rename().
The comma operator protects it from the set_file_read_write.
I did miss the error clearing.
But that just proves its too hard to read which is why we don't want this version.

> I think a helper function returning apr_status_t would be the best solution.

I have attached a helper function for comment, but as I wont have time for lots
of testing till later in the week and I can't guarantee it at the moment, I guess its
missed 1.5.3.

>
> The helper should clear the svn_error_t and return only the status. Or it
> should directly call apr to reset the attribute if that is an easier
> solution. (The cost of allocating an error is not an issue as we are
> delaying on the error anyway).
>
> Thanks,
> Bert
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org