RE: [PATCH] fix for issue #2556: support working copies on the root of a (virtual) drive

> -----Original Message-----
> From: Erik Huelsmann [mailto:ehuels@gmail.com]
> Sent: dinsdag 22 augustus 2006 23:39

> Apart from this little nit:
>
> + if ((status == APR_SUCCESS ||
> + status == APR_EINCOMPLETE) &&
> + rel_path[0] == '\0')
> + {
> + result = TRUE;
> + goto cleanup;
> + }
> +
> + cleanup:
> + if (!pool)
> + apr_pool_destroy(strpool);
> + return result;
> +}
>
>
> Which is shorter written as
>
> + if ((status == APR_SUCCESS ||
> + status == APR_EINCOMPLETE) &&
> + rel_path[0] == '\0')
> + result = TRUE;
> +
> + cleanup:
> + if (!pool)
> + apr_pool_destroy(strpool);
> + return result;
> +}
>

>I'm +1 on committing this. Note that that's only based on code review:
>I have no means of testing it here, but judging from the review, it's good.

Hm, I don't like to use fall-through, it's bound to be forgotten when
someone adds an extra line of code after the 'if' statement later. I've
checked the subversion code before writing this part and this approach seems
to be fairly common.

I'll wait with the commit for now, see if there other remarks.

Thanks for taking the time you spent on the review Erik (and enjoy the rest
of your holiday).

Lieven.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 22 23:47:32 2006