Re: [PATCH v6] generalizing sorted-array handling

From: Yann Dirson <ydirson@free.fr>
Date: Dec 30, 2010, 00:01 UTC
Message-ID: <20101230000119.GA6639@home.lan>
In-Reply-To: <7vd3p9b4d1.fsf@alter.siamese.dyndns.org>

On Fri, Dec 10, 2010 at 03:22:18PM -0800, Junio C Hamano wrote:

Show 10 quoted lines

> Yann Dirson <ydirson@altern.org> writes:
> 
> > ... I want to get my focus back to
> > bulk-rename/builk-rm patches, which will make heavy use of this API.
> 
> Final comment.  As the primary thing you want to use this is to change the
> way how the rename_dst/rename_src tables are managed, and these are both
> tables sorted by a string, I suspect a more reasonable might be to first
> updated them to use string-list API and add to that API whatever necessary
> features you might need, if any.

It sounds reasonable to build on existing stuff (furthermore, the string-list binary search is one I had missed).

Using string-lists here however will imply some tradeofs:

* the additional char* pointer in every list element is possibly not
  so high a price to pay

* using the "util" pointer for the payload will make memory management
  even more hairy (eg. "util" as a pointer to a struct which contains
  a pointer to a diff_filespec).  Convenience wrappers will be highly
  needed, and will also be required to keep calls to lookup/insert
  readable, when the elements we deal with are not strings but indeed
  the "util" stuff.

All in all, looks that the data-structure needed should have a higher focus on the "util" field than string-list has.

Features that seem to miss from string-list today (for the "dir-rename" series) include:

* custom string-comparison function (ie. prefix comparison): that
  would not be so difficult to generalize by adding a cmp_func
  parameter to get_entry_index().  That would imply changing
  widely-used API funcs like string_list_lookup() to shallow wrappers
  around variants that also take a cmp_func argument.

* lists indexed by 2 strings (bulkmove_candidates): could be replaced
  by using string-lists of string-lists instead, but I'm not sure the
  result would be that great

I still have mixed feelings about all of this.

-- 
Yann