Jeff King <p...@peff.net> wrote:
> We keep an LRU list of entries for when we need to drop
> something from an over-full cache. The list is implemented
> as a circular doubly-linked list, which is exactly what
> list.h provides. We can save a few lines by using the list.h
> macros and functions. More importantly, this makes the code
> easier to follow, as the reader sees explicit concepts like
> "list_add_tail()" instead of pointer manipulation.
> As a bonus, the list_entry() macro lets us place the lru
> pointers anywhere inside the delta_base_cache_entry struct
> (as opposed to just casting the pointer, which requires it
> at the front of the struct). This will be useful in later
> patches when we need to place other items at the front of
> the struct (e.g., our hashmap implementation requires this).
On a side note, I think we should s/list_entry/container_of/ and
use container_of for hashmap.
Linux kernel defines list_entry to use container_of,
but I'd rather avoid introducing the duality entirely.
> Signed-off-by: Jeff King <p...@peff.net>
> I think the result is much nicer, but I found list_entry() a little
> disappointing, because we lack typeof(). So you are stuck writing:
> struct delta_base_cache_entry *f =
> list_entry(p, struct delta_base_cache_entry, lru);
> I waffled on adding something like:
> LIST_ITEM(struct delta_bas_cache_entry, f, p, lru);
> to declare "f" as above. But it's getting rather magical and un-C-like.
Right. I'd rather keep the list_entry/container_of usage
identical to what developers familiar using userspace-rcu,
ccan/list, or the Linux kernel expect.
> sha1_file.c | 38 ++++++++++++++++----------------------
> 1 file changed, 16 insertions(+), 22 deletions(-)
Good to see the code reduction.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html