git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] unpack-tree.c: remove dead code

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 12, 2014, 22:24 UTC
Message-ID
<xmqqmwb94a8l.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1407878107-22850-1-git-send-email-stefanbeller@gmail.com>
Stefan Beller <stefanbeller@gmail.com> writes:
Show 33 quoted lines
> In line 1763 of unpack-tree.c we have a condition on the current tree
> 	if (current) {
> 		...
> Within this block of code we can assume current to be non NULL, hence
> the code after the statement in line 1796:
> 	if (current)
> 		return ...
>
> cannot be reached.
>
> current/newtree/oldtree are used in the
> call to reject_merge() *only* for their path aka ce->name, and they
> all point at the same name (there is no rename funkies here); hence
> "all other failures" code path should just rely on current always
> being present.
>
> All referenced lines have been introduced in the same commit
> 076b0adc (2006-07-30, read-tree: move merge functions to the library),
> which was just moving the code around.
> The outer condition on the current tree (now in line 1763) was introduced
> in c859600954df4c292e, June 2005, [PATCH] read-tree: save more user hassles during fast-forward.
> The inner condition on the current tree was introduced in
> ee6566e8d70da682ac4926d, Sept. 2005, [PATCH] Rewrite read-tree
>
> This issue was found by coverity, Id:290002
>
> Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> ---
>  unpack-trees.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>
> Did I understand you right, when changing to this one?

Something like that. I've already pushed out the original with a tentative "SQUASH???" on top for today's integration; I'll try to remember replacing them with this version.

Thanks.
Show 25 quoted lines
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index c6aa8fb..42ee84e 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1789,15 +1789,11 @@ int twoway_merge(const struct cache_entry * const *src,
>  			/* 20 or 21 */
>  			return merged_entry(newtree, current, o);
>  		}
> +		else if (o->gently) {
> +			return  -1 ;
> +		}
>  		else {
> -			/* all other failures */
> -			if (oldtree)
> -				return o->gently ? -1 : reject_merge(oldtree, o);
> -			if (current)
> -				return o->gently ? -1 : reject_merge(current, o);
> -			if (newtree)
> -				return o->gently ? -1 : reject_merge(newtree, o);
> -			return -1;
> +			reject_merge(current, o);
>  		}
>  	}
>  	else if (newtree) {
Previous: Stefan BellerNext: Jonathan Nieder
Message 4 of 15 in “unpack-tree.c: remove dead code”
  1. unpack-tree.c: remove dead codeStefan Beller, Aug 11, 2014
  2. Junio C HamanoAug 12, 2014
  3. unpack-tree.c: remove dead codeStefan Beller, Aug 12, 2014
  4. Junio C HamanoAug 12, 2014
  5. 0/3 Re: [PATCH] unpack-tree.c: remove dead codeJonathan Nieder, Aug 12, 2014
  6. 1/3 unpack-trees: simplify 'all other failures' caseJonathan Nieder, Aug 12, 2014
  7. 2/3 unpack-trees: use 'cuddled' style for if-else cascadeJonathan Nieder, Aug 13, 2014
  8. Ronnie SahlbergAug 13, 2014
  9. 3/3 checkout -m: attempt merge when deletion of path was stagedJonathan Nieder, Aug 13, 2014
  10. Junio C HamanoAug 13, 2014
  11. Junio C HamanoAug 13, 2014
  12. Junio C HamanoAug 13, 2014
  13. Johannes SixtAug 13, 2014
  14. Junio C HamanoAug 13, 2014
  15. Stefan BellerAug 13, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.