Re: [PATCH/RFC v1 2/6] remove some memcpy() and strchr() calls inside create_directories()

[Junio C Hamano <gitster@pobox.com>]

Kjetil Barvik <barvik@broadpark.no> writes:

> OK, maybe I instead should have tried to merge the function
> create_directories() with the safe_create_leading_directories() and
> *_const() functions?  What do pepople think?

Strictly speaking, the safe_create_leading_* functions are meant to work
on paths inside $GIT_DIR and are not meant for paths inside the work tree,
which is this function is about.  Their semantics are different with
respect to adjust_shared_perm().

Which means that you would either have two variants (one for work tree
paths, and another for paths inside $GIT_DIR), or a unified function that
has an argument to specify whether to run adjust_shared_perm().

HOWEVER.

That is only "strictly speaking".

A non-bare repository that is shared feels like an oximoron, but perhaps
there is a valid "shared work area" workflow that may benefit from such a
setup.

I see existing (mis)uses of the safe_create_leading_* in builtin-apply.c,
builtin-clone.c (the one that creates the work_tree, the other one is Ok),
merge-recursive.c (both call sites) that are used to touch the work tree,
but all places that create regular files in the work tree do not run
adjust_shared_perm() but simply honor the user's umask.

If we _were_ to support a "shared work area" workflow, having a unified
"create leading directory" function that always calls adjust_shared_perm()
may make sense (note that adjust_shared_perm() is a no-op in a non-shared
repository).  We then need to also call adjust_shared_perm() for codepaths
that create regular files as well, though (e.g. write_entry() in entry.c,
but there are many others).

> diff --git a/entry.c b/entry.c
> index 05aa58d..c2404ea 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -2,15 +2,19 @@
>  #include "blob.h"
>  #include "dir.h"
>  
> -static void create_directories(const char *path, const struct checkout *state)
> +static void
> +create_directories(int path_len, const char *path, const struct checkout *state)

Please do not split the line like this.

The existing sources are not laid out to allow "grep ^funcname(", nor are
we going to reformat all the files to support such a use case.

When we pass <string, length> pair to functions, I think we pass them in
the order I just wrote in all the other functions.

The micro-optimization itself makes sense to me, though.