Re: [RFC PATCH v2 2/7] object: move function to object.c

From: Glen Choo <chooglen@google.com>
Date: Aug 10, 2023, 22:36 UTC
Message-ID: <kl6ljzu28qzt.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <20230810163654.275023-2-calvinwan@google.com>

Calvin Wan <calvinwan@google.com> writes:

> While remove_or_warn() is a simple ternary operator to call two other
> wrapper functions, it creates an unnecessary dependency to object.h in
> wrapper.c. Therefore move the function to object.[ch] where the concept
> of GITLINKs is first defined.

As Junio mentioned elsewhere, I think we need to establish that wrapper.c should be free of Git-specific internals.

Show 13 quoted lines

> diff --git a/object.c b/object.c
> index 60f954194f..cb29fcc304 100644
> --- a/object.c
> +++ b/object.c
> @@ -617,3 +617,8 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
>  	FREE_AND_NULL(o->object_state);
>  	FREE_AND_NULL(o->shallow_stat);
>  }
> +
> +int remove_or_warn(unsigned int mode, const char *file)
> +{
> +	return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);
> +}

Since this function really needs S_ISGITLINK (I tried to see if we could just replace it with S_ISDIR and get the same behavior, but we can't), this really is a Git-specific thing, so yes, this should be moved out of wrapper.c.

Minor point: I think a better home might be entry.[ch], because those files care about performing changes on the worktree based on the Git-specific file modes in the index, whereas object.[ch] seems more concerned about the format of objects.