Re: [PATCH 00/22] cache cursors: an introduction

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.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html