On Mon, Jun 03, 2024 at 11:30:35AM +0200, Patrick Steinhardt wrote:
> +static int for_each_root_ref(struct files_ref_store *refs,
> + int (*cb)(const char *refname, void *cb_data),
> + void *cb_data)
> {
> struct strbuf path = STRBUF_INIT, refname = STRBUF_INIT;
> const char *dirname = refs->loose->root->name;
> struct dirent *de;
> size_t dirnamelen;
> + int ret;
> DIR *d;
Should we initialize ret to 0 here?
We set it only inside the loop over dir entries:
> @@ -357,14 +355,47 @@ static void add_root_refs(struct files_ref_store *refs,
> strbuf_addstr(&refname, de->d_name);
>
> dtype = get_dtype(de, &path, 1);
> - if (dtype == DT_REG && is_root_ref(de->d_name))
> - loose_fill_ref_dir_regular_file(refs, refname.buf, dir);
> + if (dtype == DT_REG && is_root_ref(de->d_name)) {
> + ret = cb(refname.buf, cb_data);
> + if (ret)
> + goto done;
> + }
>
> strbuf_setlen(&refname, dirnamelen);
> }
...but if the directory is empty (or only has "." files and ".lock"
files), we won't call "cb" at all, and hence won't ever set "ret".
And then at the end:
> +done:
> strbuf_release(&refname);
> strbuf_release(&path);
> closedir(d);
> + return ret;
> +}
We return uninitialized garbage.
(Sorry for the late review; this got flagged by coverity since the topic
hit 'next').
-Peff
