Re: [PATCH] Unlocking a file and pre-unlock hook failure

On Thursday 02 June 2011 05:05 PM, Philip Martin wrote:
> vijay<vijay_at_collab.net> writes:
>
>> At the end of the testcase, I thought of checking the status of the
>> working copy with expected status as "writelocked=K". But I couldn't
>> do it as the test raises exception in between, i.e., during unlock
>> operation itself.
> I don't understand this. With your patch the test is a PASS, it's also
> a PASS with the other RA layers. Why can't you test for 'K'?
>
Sorry for the confusion here. I have updated the test to check wc status
at the end.
>> ra_serf:
>>
>> The test passes with ra_serf as all kind of errors has been handled here.
>>
>> Please review the patch and respond.
>>
>>
>> Thanks& Regards,
>> Vijayaguru
>>
>> [1] https://issues.apache.org/bugzilla/show_bug.cgi?id=51297
>>
>> P.S: I will be very happy if someone from Apache's mod_dav development
>> team can take a look at the patch in [1]. Please let me know if we can
>> handle it in some other way. Eagerly awaiting for your response.
>>
>> Index: subversion/libsvn_ra_neon/lock.c
>> ===================================================================
>> --- subversion/libsvn_ra_neon/lock.c (revision 1130003)
>> +++ subversion/libsvn_ra_neon/lock.c (working copy)
>> @@ -460,7 +460,7 @@
>> _("No lock on path '%s'"
>> " (%d Bad Request)"), path, code);
>> default:
>> - break; /* Handle as error */
>> + SVN_ERR(err);
>> }
>> }
>> else
> That also fixes an error leak.
>
> However, I think that bit of code has too many return paths which is why
> the bug happened. I'd make the code simpler by moving the err
> declaration, getting rid of the "else SVN_ERR" and changing the
> SVN_NO_ERROR return to "return svn_error_return(err)". Then it's
> obvious that the error is always returned.
>
Done.

Attached the updated patch and log message.

Thanks & Regards,
Vijayaguru