git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Stefan Beller <sbeller@google.com>
Cc: Brandon Williams <bmwill@google.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id
Date: Wed, 31 May 2017 17:39:20 -0400	[thread overview]
Message-ID: <20170531213920.daqlw3s5ppmq36ww@sigill.intra.peff.net> (raw)
In-Reply-To: <20170531212920.yy3swp732rjfnzge@sigill.intra.peff.net>

On Wed, May 31, 2017 at 05:29:20PM -0400, Jeff King wrote:

> Or did you mean that diff_tree_paths() could now take an actual
> array-of-struct rather than an array-of-pointer-to-struct? That would
> drop the "parents_oid" array entirely. I think that's actually
> orthogonal to this change (the same could have been done with the
> original sha1 array), but would be a nice cleanup on top.

I took a quick look at this, but it doesn't work. This caller
(find_paths_multitree) would be happy to just pass the existing
"parents" array.

But if we change the interface to diff_tree_paths(), we also have to
change ll_diff_tree_paths() to match. And that function is called from
emit_path(), which really does assemble a list of pointers to
non-adjacent bits of memory. So it would still have to allocate, and
would now have to copy whole oids rather than just pointers.

But even worse, it seems to leave some entries as NULL. We'd need some
sentinel value (like the null sha1) to replace that.

So I think the code is already in its simplest form.

-Peff

  reply	other threads:[~2017-05-31 21:39 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-30 17:30 [PATCH 00/33] object id conversion (grep and diff) Brandon Williams
2017-05-30 17:30 ` [PATCH 01/33] notes: convert internal structures to struct object_id Brandon Williams
2017-05-30 17:30 ` [PATCH 02/33] notes: convert internal parts " Brandon Williams
2017-05-30 17:30 ` [PATCH 03/33] notes: convert for_each_note " Brandon Williams
2017-05-30 17:30 ` [PATCH 04/33] notes: make get_note return pointer " Brandon Williams
2017-07-15 18:15   ` René Scharfe
2017-07-17 17:49     ` Brandon Williams
2017-05-30 17:30 ` [PATCH 05/33] notes: convert format_display_notes " Brandon Williams
2017-05-30 17:30 ` [PATCH 06/33] builtin/notes: convert " Brandon Williams
2017-05-30 17:30 ` [PATCH 07/33] notes: convert some accessor functions " Brandon Williams
2017-05-30 17:30 ` [PATCH 08/33] grep: convert " Brandon Williams
2017-06-02  1:00   ` Junio C Hamano
2017-05-30 17:30 ` [PATCH 09/33] diff: convert get_stat_data " Brandon Williams
2017-05-30 17:30 ` [PATCH 10/33] diff: convert diff_index_show_file " Brandon Williams
2017-05-30 17:30 ` [PATCH 11/33] diff: convert diff_addremove " Brandon Williams
2017-05-30 17:30 ` [PATCH 12/33] diff: convert run_diff_files " Brandon Williams
2017-05-30 17:30 ` [PATCH 13/33] diff: convert diff_change " Brandon Williams
2017-05-30 17:30 ` [PATCH 14/33] diff: convert fill_filespec " Brandon Williams
2017-05-30 17:30 ` [PATCH 15/33] diff: convert reuse_worktree_file " Brandon Williams
2017-05-30 17:30 ` [PATCH 16/33] diff: finish conversion for prepare_temp_file " Brandon Williams
2017-05-31  0:41   ` Stefan Beller
2017-05-30 17:30 ` [PATCH 17/33] patch-ids: convert " Brandon Williams
2017-05-30 17:30 ` [PATCH 18/33] diff: convert diff_flush_patch_id " Brandon Williams
2017-05-30 17:30 ` [PATCH 19/33] combine-diff: convert diff_tree_combined " Brandon Williams
2017-05-30 17:30 ` [PATCH 20/33] combine-diff: convert find_paths_* " Brandon Williams
2017-05-31 17:49   ` Stefan Beller
2017-06-02  1:37     ` Junio C Hamano
2017-05-30 17:30 ` [PATCH 21/33] tree-diff: convert diff_root_tree_sha1 " Brandon Williams
2017-05-30 17:30 ` [PATCH 22/33] notes-merge: convert notes_merge* " Brandon Williams
2017-05-31 17:54   ` Stefan Beller
2017-05-31 22:00   ` brian m. carlson
2017-06-02  1:13     ` Junio C Hamano
2017-06-02 18:32     ` Brandon Williams
2017-05-30 17:30 ` [PATCH 23/33] notes-merge: convert merge_from_diffs " Brandon Williams
2017-05-31 18:04   ` Stefan Beller
2017-06-02  0:42     ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 24/33] notes-merge: convert find_notes_merge_pair_ps " Brandon Williams
2017-05-30 17:31 ` [PATCH 25/33] notes-merge: convert verify_notes_filepair " Brandon Williams
2017-05-31 18:09   ` Stefan Beller
2017-06-02  0:47   ` Junio C Hamano
2017-06-02 18:55     ` Brandon Williams
2017-06-02 23:49       ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 26/33] notes-merge: convert write_note_to_worktree " Brandon Williams
2017-05-30 17:31 ` [PATCH 27/33] diff-tree: convert diff_tree_sha1 " Brandon Williams
2017-05-30 17:31 ` [PATCH 28/33] builtin/diff-tree: cleanup references to sha1 Brandon Williams
2017-06-02  1:26   ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 29/33] tree-diff: convert try_to_follow_renames to struct object_id Brandon Williams
2017-05-30 17:31 ` [PATCH 30/33] tree-diff: convert diff_tree_paths " Brandon Williams
2017-05-31 18:24   ` Stefan Beller
2017-05-31 21:29     ` Jeff King
2017-05-31 21:39       ` Jeff King [this message]
2017-06-02  1:31   ` Junio C Hamano
2017-07-15 17:18   ` René Scharfe
2017-07-15 17:22     ` brian m. carlson
2017-05-30 17:31 ` [PATCH 31/33] tree-diff: convert path_appendnew to object_id Brandon Williams
2017-06-02  1:32   ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 32/33] diffcore-rename: use is_empty_blob_oid Brandon Williams
2017-05-30 17:31 ` [PATCH 33/33] diff: rename diff_fill_sha1_info to diff_fill_oid_info Brandon Williams
2017-05-31 22:06 ` [PATCH 00/33] object id conversion (grep and diff) brian m. carlson
2017-06-02 19:24   ` Brandon Williams
2017-06-02  1:34 ` Junio C Hamano
2017-06-02  5:08   ` Junio C Hamano
2017-06-02  7:19     ` Junio C Hamano
2017-06-02 18:22       ` Brandon Williams
2017-06-02 23:35         ` Junio C Hamano
2017-06-05 19:42           ` Brandon Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170531213920.daqlw3s5ppmq36ww@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sandals@crustytoothpaste.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).