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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 28, 2009, 20:36 UTC
Message-ID: <7vd4e7np5i.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1233004637-15112-3-git-send-email-barvik@broadpark.no>

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).

Show 11 quoted lines

> 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.