From: Tian Yuchen <a3205153416@gmail.com>
Date: Tue, 20 Jan 2026 15:36:25 GMT
Subject: Re: [PATCH v1][RFC] symlinks: use unsigned int for flags
Message-ID: <CA+rU_o6Mrw9ga0TST6p+8MANYaNGiKP9qud8izHL+hwxou9upA@mail.gmail.com>
In-Reply-To: <20260120152219.398999-1-a3205153416@gmail.com>

Me as total newbie to the git community (also preparing for GSoC
2026), welcome comments or any possible suggestions!

While preparing v2 to fix the return type of lstat_cache(), a broader
question regarding coding style came to mind:

I realized that even without changing the return type, the code
compiles and runs because of C's implicit integer conversion
 (since the flag values don't exceed INT_MAX).

My question is: In the Git codebase, are such "safe" implicit conversions
generally tolerated to minimize code churn, or is it considered a
best practice to strictly avoid them and match types explicitly whenever
possible?

I want to ensure I have the right standard for type strictness in
 future contributions.

Tian Yuchen <a3205153416@gmail.com> 于2026年1月20日周二 23:22写道：
>
> The 'flags' and 'track_flags' fields in symlinks.c are used
> strictly as a collection of bits (using bitwise operators including
> &, |, ~). Using a signed integer for bitmasks may lead to undefined
> behavior with shift operations and logic errors if the MSB is touched.
>
> Change these fields from 'int' to 'unsigned int' to align with C
> standards and typical usage patterns.
>
> Signed-off-by: Tian Yuchen <a3205153416@gmail.com>
> ---
>  symlinks.c | 12 ++++++------
>  symlinks.h |  4 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/symlinks.c b/symlinks.c
> index 9cc090d42c..ed63891149 100644
> --- a/symlinks.c
> +++ b/symlinks.c
> @@ -74,11 +74,11 @@ static inline void reset_lstat_cache(struct cache_def *cache)
>   */
>  static int lstat_cache_matchlen(struct cache_def *cache,
>                                 const char *name, int len,
> -                               int *ret_flags, int track_flags,
> +                               unsigned int *ret_flags, unsigned int track_flags,
>                                 int prefix_len_stat_func)
>  {
>         int match_len, last_slash, last_slash_dir, previous_slash;
> -       int save_flags, ret, saved_errno = 0;
> +       unsigned int save_flags, ret, saved_errno = 0;
>         struct stat st;
>
>         if (cache->track_flags != track_flags ||
> @@ -192,10 +192,10 @@ static int lstat_cache_matchlen(struct cache_def *cache,
>         return match_len;
>  }
>
> -static int lstat_cache(struct cache_def *cache, const char *name, int len,
> -                      int track_flags, int prefix_len_stat_func)
> +static unsigned int lstat_cache(struct cache_def *cache, const char *name, int len,
> +                      unsigned int track_flags, int prefix_len_stat_func)
>  {
> -       int flags;
> +       unsigned int flags;
>         (void)lstat_cache_matchlen(cache, name, len, &flags, track_flags,
>                         prefix_len_stat_func);
>         return flags;
> @@ -234,7 +234,7 @@ int check_leading_path(const char *name, int len, int warn_on_lstat_err)
>  static int threaded_check_leading_path(struct cache_def *cache, const char *name,
>                                        int len, int warn_on_lstat_err)
>  {
> -       int flags;
> +       unsigned int flags;
>         int match_len = lstat_cache_matchlen(cache, name, len, &flags,
>                            FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);
>         int saved_errno = errno;
> diff --git a/symlinks.h b/symlinks.h
> index 7ae3d5b856..25bf04f54f 100644
> --- a/symlinks.h
> +++ b/symlinks.h
> @@ -5,8 +5,8 @@
>
>  struct cache_def {
>         struct strbuf path;
> -       int flags;
> -       int track_flags;
> +       unsigned int flags;
> +       unsigned int track_flags;
>         int prefix_len_stat_func;
>  };
>  #define CACHE_DEF_INIT { \
> --
> 2.43.0
>