Re: [PATCH] Fix -Wmaybe-uninitialized warnings under -O0

On Fri, Apr 03, 2020 at 10:04:47AM -0400, Jeff King wrote:

I guess I should have been more clear since the -O1 and -Og locations are different. -O1 complains in filter_refs().

By the way, that function's handling of filter->kind seems very sketchy to me. It does:

  int ret = 0;
  if (!filter->kind)
                die("filter_refs: invalid type");
  else {
          /*
           * For common cases where we need only branches or remotes or tags,
           * we only iterate through those refs. If a mix of refs is needed,
           * we iterate over all refs and filter out required refs with the help
           * of filter_ref_kind().
           */
          if (filter->kind == FILTER_REFS_BRANCHES)
                  ret = for_each_fullref_in("refs/heads/", ...);
          else if (filter->kind == FILTER_REFS_REMOTES)
                  ret = for_each_fullref_in("refs/remotes/", ...);
          else if (filter->kind == FILTER_REFS_TAGS)
                  ret = for_each_fullref_in("refs/tags/", ...);
          else if (filter->kind & FILTER_REFS_ALL)
                  ret = for_each_fullref_in_pattern(filter, ...);
          if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
                  head_ref(...);
  }

So filter->kind is sometimes treated like a bitfield and sometimes not. I can set it to (ALL & DETACHED_HEAD) to get something useful, but not (BRANCHES & HEAD). The up-front check tries to complain if you didn't ask for anything, but there are other flags like INCLUDE_BROKEN that would cause "!filter->kind" to be false, but still not produce any output.

And shouldn't we be checking the return value of head_ref() like the others?

All of this is outside the scope of our current discussion, and untangling it might be messy (because it could touch the callers). I just wanted to document my findings for now. :)

-Peff