# [PATCH] cocci: drop bogus xstrdup_or_null() rule 3 messages from 2022-05-01 to 2022-05-02. Participants: Junio C Hamano, Derrick Stolee. Thread: https://gitlist.dev/t/57826 ## Junio C Hamano, 2022-05-01 05:00 Subject: [PATCH] cocci: drop bogus xstrdup_or_null() rule Message-ID: URL: https://gitlist.dev/e/xmqq1qxd6e4x.fsf%40gitster.g ``` 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 --- contrib/coccinelle/xstrdup_or_null.cocci | 8 -------- 1 file changed, 8 deletions(-) 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; @@ -- 2.36.0-233-gea8f06b421 ``` ## Derrick Stolee, 2022-05-02 13:46 Subject: Re: [PATCH] cocci: drop bogus xstrdup_or_null() rule Message-ID: <4181f814-6d8e-ff2c-6fc9-a625ac189eae@github.com> URL: https://gitlist.dev/e/4181f814-6d8e-ff2c-6fc9-a625ac189eae%40github.com In-Reply-To: ``` 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 ``` ## Junio C Hamano, 2022-05-02 17:07 Subject: Re: [PATCH] cocci: drop bogus xstrdup_or_null() rule Message-ID: URL: https://gitlist.dev/e/xmqq4k272796.fsf%40gitster.g In-Reply-To: <4181f814-6d8e-ff2c-6fc9-a625ac189eae@github.com> ``` Derrick Stolee 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. ```