From: Phillip Wood <phillip.wood123@gmail•com>
To: Patrick Steinhardt <ps@pks•im>, git@vger•kernel.org
Subject: Re: [PATCH v2 21/21] config: fix sign comparison warnings
Date: Wed, 23 Jul 2025 15:48:37 +0100 [thread overview]
Message-ID: <4ea2b50c-7277-432b-bcb6-912a0cbcb261@gmail.com> (raw)
In-Reply-To: <20250723-pks-config-wo-the-repository-v2-21-1502d60d3867@pks.im>
On 23/07/2025 15:08, Patrick Steinhardt wrote:
> There are a couple of -Wsign-compare warnings in "config.c":
>
> - `prepare_include_condition_pattern()` is returns a signed integer,
> where it either returns a negative error code or the index of the
> last dir separator in a path. That index will always be a
> non-negative number, but we cannot just change the return type to a
> `size_t` due to it being re-used as error code. This is fixed by
> splitting up concerns: the return value is only used as error code,
> and the prefix is now returned via an out-pointer. This fixes a sign
> comparison warning when comparing `text.len < prefix`,
>
Sounds good, let's see how the caller is affected ...
> @@ -239,7 +240,8 @@ static int include_by_gitdir(const struct key_value_info *kvi,
> {
> struct strbuf text = STRBUF_INIT;
> struct strbuf pattern = STRBUF_INIT;
> - int ret = 0, prefix;
> + size_t prefix;
> + int ret = 0;
> const char *git_dir;
> int already_tried_absolute = 0;
>
> @@ -250,12 +252,11 @@ static int include_by_gitdir(const struct key_value_info *kvi,
>
> strbuf_realpath(&text, git_dir, 1);
> strbuf_add(&pattern, cond, cond_len);
> - prefix = prepare_include_condition_pattern(kvi, &pattern);
> -
> -again:
> - if (prefix < 0)
> + ret = prepare_include_condition_pattern(kvi, &pattern, &prefix);
> + if (ret < 0)
> goto done;
>
> +again:
prefix is not modified below this point so moving the label down below
the error check is safe.
This looks good, thanks
Phillip
> if (prefix > 0) {
> /*
> * perform literal matching on the prefix part so that
> @@ -724,7 +725,6 @@ int git_config_from_parameters(config_fn_t fn, void *data)
> if (env) {
> unsigned long count;
> char *endp;
> - int i;
>
> count = strtoul(env, &endp, 10);
> if (*endp) {
> @@ -736,10 +736,10 @@ int git_config_from_parameters(config_fn_t fn, void *data)
> goto out;
> }
>
> - for (i = 0; i < count; i++) {
> + for (unsigned long i = 0; i < count; i++) {
> const char *key, *value;
>
> - strbuf_addf(&envvar, "GIT_CONFIG_KEY_%d", i);
> + strbuf_addf(&envvar, "GIT_CONFIG_KEY_%lu", i);
> key = getenv_safe(&to_free, envvar.buf);
> if (!key) {
> ret = error(_("missing config key %s"), envvar.buf);
> @@ -747,7 +747,7 @@ int git_config_from_parameters(config_fn_t fn, void *data)
> }
> strbuf_reset(&envvar);
>
> - strbuf_addf(&envvar, "GIT_CONFIG_VALUE_%d", i);
> + strbuf_addf(&envvar, "GIT_CONFIG_VALUE_%lu", i);
> value = getenv_safe(&to_free, envvar.buf);
> if (!value) {
> ret = error(_("missing config value %s"), envvar.buf);
> @@ -1614,13 +1614,13 @@ int config_with_options(config_fn_t fn, void *data,
>
> static void configset_iter(struct config_set *set, config_fn_t fn, void *data)
> {
> - int i, value_index;
> + int value_index;
> struct string_list *values;
> struct config_set_element *entry;
> struct configset_list *list = &set->list;
> struct config_context ctx = CONFIG_CONTEXT_INIT;
>
> - for (i = 0; i < list->nr; i++) {
> + for (size_t i = 0; i < list->nr; i++) {
> entry = list->items[i].e;
> value_index = list->items[i].value_index;
> values = &entry->value_list;
> @@ -2470,10 +2470,11 @@ static ssize_t write_pair(int fd, const char *key, const char *value,
> */
> static void maybe_remove_section(struct config_store_data *store,
> size_t *begin_offset, size_t *end_offset,
> - int *seen_ptr)
> + unsigned *seen_ptr)
> {
> size_t begin;
> - int i, seen, section_seen = 0;
> + int section_seen = 0;
> + unsigned int i, seen;
>
> /*
> * First, ensure that this is the first key, and that there are no
> @@ -2716,7 +2717,8 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
> } else {
> struct stat st;
> size_t copy_begin, copy_end;
> - int i, new_line = 0;
> + unsigned i;
> + int new_line = 0;
> struct config_options opts;
>
> if (!value_pattern)
>
next prev parent reply other threads:[~2025-07-23 14:48 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-17 10:49 [PATCH 00/21] config: remove use of `the_repository` Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 01/21] config: drop `git_config()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 02/21] config: drop `git_config_clear()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 03/21] config: drop `git_config_get()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 04/21] config: drop `git_config_get_value()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 05/21] " Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 06/21] config: drop `git_config_get_string_multi()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 07/21] config: drop `git_config_get_string()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 08/21] " Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 09/21] config: drop `git_config_get_int()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 10/21] config: drop `git_config_get_ulong()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 11/21] config: drop `git_config_get_bool()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 12/21] config: drop `git_config_set_in_file()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 13/21] config: drop `git_config_set_gently()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 14/21] config: drop `git_config_set()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 15/21] config: drop `git_config_set_in_file_gently()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 16/21] config: drop `git_config_set_multivar_in_file_gently()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 17/21] config: drop `git_config_get_multivar_gently()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 18/21] config: drop `git_config_set_multivar()` wrapper Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 19/21] config: remove unused `the_repository` wrappers Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 20/21] config: move Git config parsing into "environment.c" Patrick Steinhardt
2025-07-17 10:49 ` [PATCH 21/21] config: fix sign comparison warnings Patrick Steinhardt
2025-07-23 9:38 ` Phillip Wood
2025-07-23 13:38 ` Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 00/21] config: remove use of `the_repository` Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 01/21] config: drop `git_config()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 02/21] config: drop `git_config_clear()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 03/21] config: drop `git_config_get()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 04/21] config: drop `git_config_get_value()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 05/21] " Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 06/21] config: drop `git_config_get_string_multi()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 07/21] config: drop `git_config_get_string()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 08/21] " Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 09/21] config: drop `git_config_get_int()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 10/21] config: drop `git_config_get_ulong()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 11/21] config: drop `git_config_get_bool()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 12/21] config: drop `git_config_set_in_file()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 13/21] config: drop `git_config_set_gently()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 14/21] config: drop `git_config_set()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 15/21] config: drop `git_config_set_in_file_gently()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 16/21] config: drop `git_config_set_multivar_in_file_gently()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 17/21] config: drop `git_config_get_multivar_gently()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 18/21] config: drop `git_config_set_multivar()` wrapper Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 19/21] config: remove unused `the_repository` wrappers Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 20/21] config: move Git config parsing into "environment.c" Patrick Steinhardt
2025-07-23 14:08 ` [PATCH v2 21/21] config: fix sign comparison warnings Patrick Steinhardt
2025-07-23 14:48 ` Phillip Wood [this message]
2025-07-23 15:29 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4ea2b50c-7277-432b-bcb6-912a0cbcb261@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox