{"thread":{"id":"64843","subject":"[PATCH v2] symlinks: use unsigned int for flags","startedAt":"2026-01-21T16:26:51Z","lastAt":"2026-02-16T17:20:43Z","messageCount":3,"participants":["Tian Yuchen","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"534362","messageId":"20260121162640.424126-1-a3205153416@gmail.com","threadId":"64843","inReplyTo":null,"subject":"[PATCH v2] symlinks: use unsigned int for flags","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-01-21T16:26:40Z","receivedAt":"2026-01-21T16:26:51Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"The 'flags' and 'track_flags' fields in symlinks.c are used\nstrictly as a collection of bits (using bitwise operators including\n&, |, ~). Using a signed integer for bitmasks may lead to undefined\nbehavior with shift operations and logic errors if the MSB is touched.\n\nChange these fields from 'int' to 'unsigned int' to align with C\nstandards and typical usage patterns.\n\nSigned-off-by: Tian Yuchen <a3205153416@gmail.com>\n\n---\nChanges in v2:\n\nDecouple definition of 'ret' and 'saved_errno' from 'save_flags'.\n'ret' captures the return value of lstat() which can be -1, so it\nmust remain signed. Same applies to 'saved_errno'.\n\n(Thanks to Patrick Steinhardt for spotting this)\n---\n symlinks.c | 13 +++++++------\n symlinks.h |  4 ++--\n 2 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/symlinks.c b/symlinks.c\nindex 9cc090d42c..9e01ab3bc8 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -74,11 +74,12 @@ static inline void reset_lstat_cache(struct cache_def *cache)\n  */\n static int lstat_cache_matchlen(struct cache_def *cache,\n \t\t\t\tconst char *name, int len,\n-\t\t\t\tint *ret_flags, int track_flags,\n+\t\t\t\tunsigned int *ret_flags, unsigned int track_flags,\n \t\t\t\tint prefix_len_stat_func)\n {\n \tint match_len, last_slash, last_slash_dir, previous_slash;\n-\tint save_flags, ret, saved_errno = 0;\n+\tunsigned int save_flags;\n+\tint ret, saved_errno = 0;\n \tstruct stat st;\n \n \tif (cache->track_flags != track_flags ||\n@@ -192,10 +193,10 @@ static int lstat_cache_matchlen(struct cache_def *cache,\n \treturn match_len;\n }\n \n-static int lstat_cache(struct cache_def *cache, const char *name, int len,\n-\t\t       int track_flags, int prefix_len_stat_func)\n+static unsigned int lstat_cache(struct cache_def *cache, const char *name, int len,\n+\t\t       unsigned int track_flags, int prefix_len_stat_func)\n {\n-\tint flags;\n+\tunsigned int flags;\n \t(void)lstat_cache_matchlen(cache, name, len, &flags, track_flags,\n \t\t\tprefix_len_stat_func);\n \treturn flags;\n@@ -234,7 +235,7 @@ int check_leading_path(const char *name, int len, int warn_on_lstat_err)\n static int threaded_check_leading_path(struct cache_def *cache, const char *name,\n \t\t\t\t       int len, int warn_on_lstat_err)\n {\n-\tint flags;\n+\tunsigned int flags;\n \tint match_len = lstat_cache_matchlen(cache, name, len, &flags,\n \t\t\t   FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n \tint saved_errno = errno;\ndiff --git a/symlinks.h b/symlinks.h\nindex 7ae3d5b856..25bf04f54f 100644\n--- a/symlinks.h\n+++ b/symlinks.h\n@@ -5,8 +5,8 @@\n \n struct cache_def {\n \tstruct strbuf path;\n-\tint flags;\n-\tint track_flags;\n+\tunsigned int flags;\n+\tunsigned int track_flags;\n \tint prefix_len_stat_func;\n };\n #define CACHE_DEF_INIT { \\\n-- \n2.43.0\n\n"},{"id":"534368","messageId":"xmqqzf66u9jj.fsf@gitster.g","threadId":"64843","inReplyTo":"20260121162640.424126-1-a3205153416@gmail.com","subject":"Re: [PATCH v2] symlinks: use unsigned int for flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-21T18:04:32Z","receivedAt":"2026-01-21T18:04:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen <a3205153416@gmail.com> writes:\n\n> The 'flags' and 'track_flags' fields in symlinks.c are used\n> strictly as a collection of bits (using bitwise operators including\n> &, |, ~). Using a signed integer for bitmasks may lead to undefined\n> behavior with shift operations and logic errors if the MSB is touched.\n\nWhich we do not do, so the \"signed can lead to bugs\" is a valid\nconcern and moving to unsigned is a good mitigation, but ...\n\n>\n> Change these fields from 'int' to 'unsigned int' to align with C\n> standards and typical usage patterns.\n\n... I'd tone it down a bit by replacing \"aling with C standards and\ntypical\" with \"match our\", if I were writing this.\n\n>\n> Signed-off-by: Tian Yuchen <a3205153416@gmail.com>\n>\n> ---\n> Changes in v2:\n>\n> Decouple definition of 'ret' and 'saved_errno' from 'save_flags'.\n> 'ret' captures the return value of lstat() which can be -1, so it\n> must remain signed. Same applies to 'saved_errno'.\n>\n> (Thanks to Patrick Steinhardt for spotting this)\n\nYes, indeed.  Thanks.\n"},{"id":"536132","messageId":"20260216172028.140525-1-a3205153416@gmail.com","threadId":"64843","inReplyTo":"xmqqzf66u9jj.fsf@gitster.g","subject":"[PATCH v3 1/1] symlinks: use unsigned int for flags","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-16T17:20:28Z","receivedAt":"2026-02-16T17:20:43Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"The 'flags' and 'track_flags' fields in symlinks.c are used\nstrictly as a collection of bits (using bitwise operators including\n&, |, ~). Using a signed integer for bitmasks may lead to undefined\nbehavior with shift operations and logic errors if the MSB is touched.\n\nChange these fields from 'int' to 'unsigned int' to match our usage\npatterns.\n\nSigned-off-by: Tian Yuchen <a3205153416@gmail.com>\n\n---\nChanges in v3:\n\nThe commit message.\n---\n symlinks.c | 13 +++++++------\n symlinks.h |  4 ++--\n 2 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/symlinks.c b/symlinks.c\nindex 9cc090d42c..9e01ab3bc8 100644\n--- a/symlinks.c\n+++ b/symlinks.c\n@@ -74,11 +74,12 @@ static inline void reset_lstat_cache(struct cache_def *cache)\n  */\n static int lstat_cache_matchlen(struct cache_def *cache,\n \t\t\t\tconst char *name, int len,\n-\t\t\t\tint *ret_flags, int track_flags,\n+\t\t\t\tunsigned int *ret_flags, unsigned int track_flags,\n \t\t\t\tint prefix_len_stat_func)\n {\n \tint match_len, last_slash, last_slash_dir, previous_slash;\n-\tint save_flags, ret, saved_errno = 0;\n+\tunsigned int save_flags;\n+\tint ret, saved_errno = 0;\n \tstruct stat st;\n \n \tif (cache->track_flags != track_flags ||\n@@ -192,10 +193,10 @@ static int lstat_cache_matchlen(struct cache_def *cache,\n \treturn match_len;\n }\n \n-static int lstat_cache(struct cache_def *cache, const char *name, int len,\n-\t\t       int track_flags, int prefix_len_stat_func)\n+static unsigned int lstat_cache(struct cache_def *cache, const char *name, int len,\n+\t\t       unsigned int track_flags, int prefix_len_stat_func)\n {\n-\tint flags;\n+\tunsigned int flags;\n \t(void)lstat_cache_matchlen(cache, name, len, &flags, track_flags,\n \t\t\tprefix_len_stat_func);\n \treturn flags;\n@@ -234,7 +235,7 @@ int check_leading_path(const char *name, int len, int warn_on_lstat_err)\n static int threaded_check_leading_path(struct cache_def *cache, const char *name,\n \t\t\t\t       int len, int warn_on_lstat_err)\n {\n-\tint flags;\n+\tunsigned int flags;\n \tint match_len = lstat_cache_matchlen(cache, name, len, &flags,\n \t\t\t   FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);\n \tint saved_errno = errno;\ndiff --git a/symlinks.h b/symlinks.h\nindex 7ae3d5b856..25bf04f54f 100644\n--- a/symlinks.h\n+++ b/symlinks.h\n@@ -5,8 +5,8 @@\n \n struct cache_def {\n \tstruct strbuf path;\n-\tint flags;\n-\tint track_flags;\n+\tunsigned int flags;\n+\tunsigned int track_flags;\n \tint prefix_len_stat_func;\n };\n #define CACHE_DEF_INIT { \\\n-- \n2.43.0\n\n"}]}