On Sat, May 12, 2018 at 05:07:48AM -0400, Jeff King wrote:
> So no, it wouldn't work to directly store depths with the code as
> written. I'm not sure if the depth can ever be 0. If not, then it would
> be a suitable sentinel as:
>
> int *slot = commit_depth_at(&depths, p->item);
> if (!*slot || cur_depth < *slot)
> *slot = cur_depth;
>
> But somebody would have to dig into the possible values of cur_depth
> there (which would make sense to do as a separate patch anyway, since
> the point of this is to be a direct conversion).
By the way, one other approach if xcalloc() doesn't produce a good
sentinel is to use a data type that does. ;) E.g., something like this
should work:
struct depth {
unsigned valid:1;
int value;
};
define_commit_slab(commit_depth, struct depth);
...
struct depth *slot = commit_depth_at(&depths, p->item);
if (!slot->valid || cur_depth < slot->value) {
slot->value = cur_depth;
slot->valid = 1;
}
That wastes an extra 4 bytes per slot over storing an int directly, but
it's the same as storing an 8-byte pointer, plus you avoid the storage
and runtime overhead of malloc.
I actually wonder if we could wrap commit_slab with a variant that
stores the sentinel data itself, to make this pattern easier. I.e.,
something like:
#define define_commit_slab_sentinel(name, type) \
struct name##_value { \
unsigned valid:1; \
type value; \
}; \
define_commit_slab(name, struct name##_value)
and some matching "peek" and "at" functions to manipulate value
directly.
I think the end result would be nicer, but it's turning into a little
bit of a rabbit hole. So I don't mind going with your direct conversion
here for now.
-Peff
