From: Chuck Lever <cel@citi.umich.edu>
Date: Mon, 12 Sep 2005 23:59:49 GMT
Subject: Re: [PATCH 00/22] cache cursors: an introduction
Message-ID: <43261675.10905@citi.umich.edu>
In-Reply-To: <7vaciiawrm.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> I have a couple of comments on the API, though.
> 
> * Doesn't function to be applied usually want to have its own
>   data when passed to walk, maybe something like this?
> 
>   typedef int (*cache_iterator_fn_t) (struct cache_cursor *cc,
> 			 struct cache_entry *ce, void *udata);
>   static inline int walk_cache(cache_iterator_fn_t func, void *udata)
>   {
>           struct cache_cursor cc;
> 
>           init_cc(&cc);
>           while (!cache_eof(&cc)) {
>                   int status = func(&cc, cc_to_ce(&cc), udata);
>                   if (status < 0)
>                           return status;
>           }
>           return 0;
>   }
> 
>   This was a question I had when I read [PATCH 01/22] before
>   reading the rest of the patches, but the actual conversion
>   does not seem to find much need for it.  A new global variable
>   pathspec is introduced to pass information the API is unable
>   to pass to diff_one() in diff-index.c, which may be a sign
>   that an extra "user data" parameter might help.  Your call.

heh.  well, i had something like this earlier, but i know linus doesn't 
like void *, and it was really kind of ugly.  and as you observed, it's 
used so rarely.  so i just decided to drop it.

> * It may make sense to give another param to describe which
>   cache the caller is talking about so that we can later have
>   more than one cache at the same time:
> 
>   struct cache {
>       struct cache_entry **cache_array;
>       unsigned int nr;
>       unsigned int alloc;
>       unsigned int cache_changed;
>   };
>   struct cache active_cache;
> 
>   and use it like this:
> 
>   static inline struct cache_entry *cc_to_ce(struct cache_cursor *cc,
>                                              struct cache *cache)
>   {
>           return cache->cache_array[cc->pos];
>   }
> 
>   We could argue that this should be left for later rounds.  On
>   the other hand, we will be changing all the cc_* function call
>   sites during that round, which is by definition the places you
>   are touching in this round anyway.

actually this is simple to add now.  i'll give it a shot (and fix up 
write_cache to use it).

btw, with daniel's changes i don't see where we're using 
active_cache_changed any more.


begin:vcard
fn:Chuck Lever
n:Lever;Charles
org:Network Appliance, Incorporated;Linux NFS Client Development
adr:535 West William Street, Suite 3100;;Center for Information Technology Integration;Ann Arbor;MI;48103-4943;USA
email;internet:cel@citi.umich.edu
title:Member of Technical Staff
tel;work:+1 734 763-4415
tel;fax:+1 734 763 4434
tel;home:+1 734 668-1089
x-mozilla-html:FALSE
url:http://www.monkey.org/~cel/
version:2.1
end:vcard