Re: [PATCH/RFC v3 4/9] unlink_entry(): introduce schedule_dir_for_removal()

[Kjetil Barvik <barvik@broadpark.no>]

Junio C Hamano <gitster@pobox.com> writes:

> Kjetil Barvik <barvik@broadpark.no> writes:
>
>> Since the unlink_entry() function is called with alphabetically sorted
>> names, this new function end up being very effective to avoid
>> unnecessary calls to rmdir().  In some cases over 95% of all calls to
>> rmdir() is removed with this patch.
>
> I first feared that this will badly break D/F conflicting cases where we
> remove a file D/F that is the last remaining file in directory D and rely
> on the successful removal of the directory to create a file D, but the
> check_updates() function already is structured to allow this late removal
> easily.
>
> I think the patch is sensible modulo minor nits.

  Yes, if the 2 for-loop's inside check_updates() is rewritten into 1
  for-loop, then you could end up with such a conflict.  Should I add a
  comment about this inside check_updates()?

>  * The new global path_buf/path_len in unpack-trees.c are named too
>    generic.  It is not just a buffer anybody can use for anything, but
>    is used to keep track for a specific purpose of doing something, so
>    please name them after that something.

   Already done in v3 (this version), since the functions
   schedule_dir_for_removal() and remove_scheduled_dirs() is now
   implemented in the symlinks.c file, where I have made the following:

static struct removal_def {
	char path[PATH_MAX];
	int len;
} removal;

>  * "#define ALL_DIRS 0" is useless, given the way how the parameter to
>    do_remove_scheduled_dirs() is named;

  Ok, I can do a rebase of this patch (4/9) and resend the patches.

  -- kjetil