| Message ID | RFC-patch-06.15-2d04035d7aa-20220603T183608Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix GCC -fanalyzer warnings & add -fanalyzer DEVOPTS mode | expand |
Am 03.06.22 um 20:37 schrieb Ævar Arnfjörð Bjarmason: > Adjust code originally added in 5339bdad96a (ref-filter: introduce > remote_ref_atom_parser(), 2016-02-17) to BUG() out rather than > potentially segfault if we get a NULL refname here. > > As noted by GCC v12's -fanalyzer that will happen if this follows the > "refname = NULL" branch added in cc72385fe35 (for-each-ref: let > upstream/push optionally report the remote name, 2017-10-05). > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > ref-filter.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/ref-filter.c b/ref-filter.c > index 2413f889f48..91aa8e89268 100644 > --- a/ref-filter.c > +++ b/ref-filter.c > @@ -1644,7 +1644,9 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, > struct branch *branch, const char **s) > { > int num_ours, num_theirs; > - if (atom->u.remote_ref.option == RR_REF) > + if (atom->u.remote_ref.option == RR_REF && !refname) > + BUG("must get refname with [...]remote_ref.option == RR_REF"); > + else if (atom->u.remote_ref.option == RR_REF) The related code not shown here looks tricky. IIUC refname cannot be NULL if option is RR_REF, because remote_ref_atom_parser() only sets .push_remote for other option values and only if that's set we can get a NULL refname passed to fill_remote_ref_details(). Right? Do we need to have two different ways to indicate remote-ness? Having just one like after the patch below should reduce the risk of confusion. > *s = show_ref(&atom->u.remote_ref.refname, refname); > else if (atom->u.remote_ref.option == RR_TRACK) { > if (stat_tracking_info(branch, &num_ours, &num_theirs, diff --git a/ref-filter.c b/ref-filter.c index 2413f889f4..260257b92c 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -183,7 +183,7 @@ static struct used_atom { RR_REF, RR_TRACK, RR_TRACKSHORT, RR_REMOTE_NAME, RR_REMOTE_REF } option; struct refname_atom refname; - unsigned int nobracket : 1, push : 1, push_remote : 1; + unsigned int nobracket : 1, push : 1; } remote_ref; struct { enum { C_BARE, C_BODY, C_BODY_DEP, C_LENGTH, C_LINES, @@ -295,10 +295,8 @@ static int remote_ref_atom_parser(struct ref_format *format, struct used_atom *a atom->u.remote_ref.nobracket = 1; else if (!strcmp(s, "remotename")) { atom->u.remote_ref.option = RR_REMOTE_NAME; - atom->u.remote_ref.push_remote = 1; } else if (!strcmp(s, "remoteref")) { atom->u.remote_ref.option = RR_REMOTE_REF; - atom->u.remote_ref.push_remote = 1; } else { atom->u.remote_ref.option = RR_REF; if (refname_atom_parser_internal(&atom->u.remote_ref.refname, @@ -1891,7 +1889,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) continue; branch = branch_get(branch_name); - if (atom->u.remote_ref.push_remote) + if (atom->u.remote_ref.option == RR_REMOTE_NAME || + atom->u.remote_ref.option == RR_REMOTE_REF) refname = NULL; else { refname = branch_get_push(branch, NULL);
diff --git a/ref-filter.c b/ref-filter.c index 2413f889f48..91aa8e89268 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1644,7 +1644,9 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, struct branch *branch, const char **s) { int num_ours, num_theirs; - if (atom->u.remote_ref.option == RR_REF) + if (atom->u.remote_ref.option == RR_REF && !refname) + BUG("must get refname with [...]remote_ref.option == RR_REF"); + else if (atom->u.remote_ref.option == RR_REF) *s = show_ref(&atom->u.remote_ref.refname, refname); else if (atom->u.remote_ref.option == RR_TRACK) { if (stat_tracking_info(branch, &num_ours, &num_theirs,
Adjust code originally added in 5339bdad96a (ref-filter: introduce remote_ref_atom_parser(), 2016-02-17) to BUG() out rather than potentially segfault if we get a NULL refname here. As noted by GCC v12's -fanalyzer that will happen if this follows the "refname = NULL" branch added in cc72385fe35 (for-each-ref: let upstream/push optionally report the remote name, 2017-10-05). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- ref-filter.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)