| Message ID | xmqq1qxd6e4x.fsf@gitster.g (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 08bdd3a1851cca1cebed0c5a26d1d4fcd5a73d16 |
| Headers | show |
| Series | cocci: drop bogus xstrdup_or_null() rule | expand |
On 5/1/2022 1:00 AM, Junio C Hamano wrote: > 13092a91 (cocci: refactor common patterns to use xstrdup_or_null(), > 2016-10-12) introduced a rule to rewrite this conditional call to > xstrdup(E) and an assignment to variable V: > > - if (E) > - V = xstrdup(E); > > into an unconditional call to xstrdup_or_null(E) and an assignment > to variable V: > > + V = xstrdup_or_null(E); > > which is utterly bogus. The original code may already have an > acceptable value in V and the conditional assignment may be to > improve the value already in V with a copy of a better value E when > (and only when) E is not NULL. Yes, this makes sense. > The rewritten construct unconditionally discards the existing value > of V and replaces it with a copy of E, even when E is NULL, which > changes the meaning of the program. > > By the way, if it were > > -if (E && !V) > - V = xstrdup(E); > +V = xstrdup_or_null(E); I think you mean if it were -if (E && !V) - V = xstrdup(E); +if (!V) + V = xstrdup_or_null(E); or -if (E && !V) - V = xstrdup(E); +free(V); +V = xstrdup_or_null(E); But yes, there is no preimage matching this pattern, so it doesn't matter. Dropping the rule makes the most sense. Thanks, -Stolee
Derrick Stolee <derrickstolee@github.com> writes: > I think you mean if it were > ... Please disregard. That one was completely bogus. > Dropping the rule makes the most sense. Yup. ThHanks for a doze of sanity.
diff --git a/contrib/coccinelle/xstrdup_or_null.cocci b/contrib/coccinelle/xstrdup_or_null.cocci index 8e05d1ca4b..9c1d2939b6 100644 --- a/contrib/coccinelle/xstrdup_or_null.cocci +++ b/contrib/coccinelle/xstrdup_or_null.cocci @@ -1,11 +1,3 @@ -@@ -expression E; -expression V; -@@ -- if (E) -- V = xstrdup(E); -+ V = xstrdup_or_null(E); - @@ expression E; @@
13092a91 (cocci: refactor common patterns to use xstrdup_or_null(), 2016-10-12) introduced a rule to rewrite this conditional call to xstrdup(E) and an assignment to variable V: - if (E) - V = xstrdup(E); into an unconditional call to xstrdup_or_null(E) and an assignment to variable V: + V = xstrdup_or_null(E); which is utterly bogus. The original code may already have an acceptable value in V and the conditional assignment may be to improve the value already in V with a copy of a better value E when (and only when) E is not NULL. The rewritten construct unconditionally discards the existing value of V and replaces it with a copy of E, even when E is NULL, which changes the meaning of the program. By the way, if it were -if (E && !V) - V = xstrdup(E); +V = xstrdup_or_null(E); it would probably have been correct. But there is no existing code that would have been improved by such a rule, so let's just remove the bogus one without replacing with the more specific one. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- contrib/coccinelle/xstrdup_or_null.cocci | 8 -------- 1 file changed, 8 deletions(-)