You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by VK Sameer <sa...@collab.net> on 2005/02/28 04:15:09 UTC

Re: [PATCH] issue 2154 - svn blame on a directory over DAV does not return gracefully

On Mon, 2005-02-28 at 03:09, Peter N. Lundblad wrote:
>
> FYI, this code was shamelessly stolen (guess by whome:-) from another
> report implementation. I think this bug is present in several places (the
> log code, get_locations), which will lead to ugly errors if some of the
> underlying repos functions fail for some reason. It would be great if you
> fixed those in the same way!

Adding the fix is not a problem, but checking this out showed
inconsistencies in the if expression used at the 'cleanup:' label.
update.c has this:
if ((!derr) && ap_fflush ...)

while file_revs.c, version.c, and log.c have this:

if ((ap_fflush(...) && (!derr))

Checking for !derr first would let 'goto cleanup:' be retained in
file_revs.c, but I don't know if that is a reasonable solution in all
cases. version.c doesn't use the goto idiom, so it doesn't seem to need
this fix. Comments?

Regards
Sameer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] issue 2154 - svn blame on a directory over DAV does not return gracefully

Posted by VK Sameer <sa...@collab.net>.
Attached is a patch based on Justin's comment.

'make davcheck' gave some errors in different test cases, merge_tests.py
once, but they didn't recur when run separately, leading me to think it
may have been due to my httpd performance configuration. 'make check'
passed.

Regards
Sameer

Re: [PATCH] issue 2154 - svn blame on a directory over DAV does not return gracefully

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
VK Sameer wrote:

> Checking for !derr first would let 'goto cleanup:' be retained in
> file_revs.c, but I don't know if that is a reasonable solution in all
> cases. version.c doesn't use the goto idiom, so it doesn't seem to need
> this fix. Comments?

I believe that checking for !derr first is reasonable as the flush is only
an optimization anyway.  So, if we got an error that we're going to
return, then it isn't critical for us to flush as its not likely we wrote
anything at that point.  -- justin


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org