On Thu, Jul 09, 2015 at 03:33:46PM +0000, Sebastian Schuberth wrote:
> @@ -174,19 +175,17 @@ static char *guess_dir_name(const char *repo, int
> is_bundle, int is_bare)
> * Strip .{bundle,git}.
> */
> if (is_bundle) {
> - if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
> - end -= 7;
> + strip_suffix(start, ".bundle", &len);
> } else {
> - if (end - start > 4 && !strncmp(end - 4, ".git", 4))
> - end -= 4;
> + strip_suffix(start, ".git", &len);
> }
Yay, always glad to see complicated string handling like this go away.
As the resulting conditional blocks are one-liners, I think you can drop
the curly braces, which will match our usual style:
if (is_bundle)
strip_suffix(start, ".bundle", &len);
else
strip_suffix(start, ".git", &len);
If you wanted to get really fancy, I think you could put a ternary
operator in the middle of the strip_suffix call. That makes it clear
that "len" is set in all code paths, but I think some people find
ternary operators unreadable. :)
> if (is_bare) {
> struct strbuf result = STRBUF_INIT;
> - strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
> + strbuf_addf(&result, "%.*s.git", len, start);
> dir = strbuf_detach(&result, NULL);
This one can also be simplified using xstrfmt to:
if (is_bare)
dir = xstrfmt("%.*s.git", len, start);
Do we still need to cast "len" to an int to use it with "%.*" (it is
defined by the standard as an int, not a size_t)?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
