Re: [PATCH 41/68] init: use strbufs to store paths

From: Jeff King <peff@peff.net>
Date: Oct 5, 2015, 03:41 UTC
Message-ID: <20151005034126.GA5639@sigill.intra.peff.net>
In-Reply-To: <5610C7C3.807@web.de>

On Sun, Oct 04, 2015 at 08:31:31AM +0200, Torsten Bögershausen wrote:

Show 9 quoted lines

> > That is the original signature, before my sprintf series. I do not mind
> > leaving that as-is, and simply cleaning up probe_utf8_pathname_composition
> > by using a strbuf internally there. Though I have to wonder if it even
> > needs us to pass _anything_ at that point. It could just call
> > git_path_buf("config%s", auml_nfd) itself. The whole reason to pass
> > anything was to let it reuse the buffer the caller had.
> >
> > -Peff
> Makes sense, here is V2:

Yeah, I think this is much nicer.

And because it decouples the interface between init-db.c and the precompose code, it is easy to do it as a separate patch before the init-db one.

Show 9 quoted lines

> diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
> index b4dd3c7..64b85f2 100644
> --- a/compat/precompose_utf8.c
> +++ b/compat/precompose_utf8.c
> @@ -8,6 +8,7 @@
>  #include "cache.h"
>  #include "utf8.h"
>  #include "precompose_utf8.h"
> +#include "strbuf.h"

I think this is actually redundant; it is part of cache.h included above (and the precompose_utf8.h header file does not need to care anymore, since the strbuf is not part of the interface).

Show 8 quoted lines

> -void probe_utf8_pathname_composition(struct strbuf *path)
> +void probe_utf8_pathname_composition(void)
>  {
> +    struct strbuf sbuf = STRBUF_INIT;
>      static const char *auml_nfc = "\xc3\xa4";
>      static const char *auml_nfd = "\x61\xcc\x88";
> -    size_t baselen = path->len;
> +    const char *path;

I don't think we need this separate "path"; we can just access the strbuf directly (that makes the diff a little noisier, but I think the end result is simpler).

Show 10 quoted lines

> diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
> index 7fc7be5..a94e7c4 100644
> --- a/compat/precompose_utf8.h
> +++ b/compat/precompose_utf8.h
> @@ -27,7 +27,7 @@ typedef struct {
>  } PREC_DIR;
>  
>  void precompose_argv(int argc, const char **argv);
> -void probe_utf8_pathname_composition(struct strbuf *path);
> +void probe_utf8_pathname_composition(void);

I think we need a similar fix for the compat macro to build on non-Mac platforms.

Here's a mini-series I came up with, which I hope is polished enough for Junio to apply as a drop-in replacement for the "init: use strbufs" patch from my original series. I compiled-tested it on Linux, with and without precompose_utf8.o support hacked in. I don't have access to an OS X machine to test on, so I'd appreciate confirmation that t3910 still passes there.

  [1/3]: precompose_utf8: drop unused variable
  [2/3]: probe_utf8_pathname_composition: use internal strbuf
  [3/3]: init: use strbufs to store paths

-Peff