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)
