| Message ID | 20190121223216.66659-2-sxenos@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Mon, Jan 21, 2019 at 2:32 PM <sxenos@google.com> wrote: > > evolve: Implement oid_array_contains_nondestructive I'd think I would word this sha1-array: implement oid_array_contains_nondestructive as for this patch it is not relevant what we we'll be using it for later, but rather that it touches the oidset class? > From: Stefan Xenos <sxenos@gmail.com> > > Implement a "contains_nondestructive" function for oid_array that won't > sort the array if it is unsorted. This can be used to test containment in > the rare situations where the array order matters. > > The function has intentionally been given a name that is more cumbersome > than the "lookup" function, which is what most callers will will want > in most situations. What about naming it oid_array_linear_lookup instead? That would still have the common "lookup" in the name and the "linear" should be enough to scare away the casual user. The non-destructive sounds scary. Missing sign off
On Tue, Jan 22, 2019 at 11:51 AM Stefan Beller <sbeller@google.com> wrote: > > On Mon, Jan 21, 2019 at 2:32 PM <sxenos@google.com> wrote: > > > > > evolve: Implement oid_array_contains_nondestructive > > I'd think I would word this > > sha1-array: implement oid_array_contains_nondestructive Good point. Done. > as for this patch it is not relevant what we we'll be using it for > later, but rather that it touches the oidset class? > > > From: Stefan Xenos <sxenos@gmail.com> > > > > Implement a "contains_nondestructive" function for oid_array that won't > > sort the array if it is unsorted. This can be used to test containment in > > the rare situations where the array order matters. > > > > The function has intentionally been given a name that is more cumbersome > > than the "lookup" function, which is what most callers will will want > > in most situations. > > What about naming it oid_array_linear_lookup instead? > That would still have the common "lookup" in the name and > the "linear" should be enough to scare away the casual > user. The non-destructive sounds scary. It probably shouldn't contain the word "lookup" since the lookup method returns an index and this new method returns true/false. I changed it to oid_array_readonly_contains. > Missing sign off Thanks!
diff --git a/sha1-array.c b/sha1-array.c index b94e0ec0f5..466937d73d 100644 --- a/sha1-array.c +++ b/sha1-array.c @@ -26,6 +26,21 @@ static const unsigned char *sha1_access(size_t index, void *table) return array[index].hash; } +int oid_array_contains_nondestructive(const struct oid_array* array, + const struct object_id* oid) +{ + int i; + if (array->sorted) { + return sha1_pos(oid->hash, array->oid, array->nr, sha1_access) >= 0; + } + for (i = 0; i < array->nr; i++) { + if (hashcmp(array->oid[i].hash, oid->hash) == 0) { + return 1; + } + } + return 0; +} + int oid_array_lookup(struct oid_array *array, const struct object_id *oid) { if (!array->sorted) diff --git a/sha1-array.h b/sha1-array.h index 232bf95017..a887e7e4eb 100644 --- a/sha1-array.h +++ b/sha1-array.h @@ -13,6 +13,8 @@ struct oid_array { void oid_array_append(struct oid_array *array, const struct object_id *oid); int oid_array_lookup(struct oid_array *array, const struct object_id *oid); void oid_array_clear(struct oid_array *array); +int oid_array_contains_nondestructive(const struct oid_array* array, + const struct object_id* oid); typedef int (*for_each_oid_fn)(const struct object_id *oid, void *data); diff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c index ad5e69f9d3..127a4c0b51 100644 --- a/t/helper/test-sha1-array.c +++ b/t/helper/test-sha1-array.c @@ -25,10 +25,16 @@ int cmd__sha1_array(int argc, const char **argv) if (get_oid_hex(arg, &oid)) die("not a hexadecimal SHA1: %s", arg); printf("%d\n", oid_array_lookup(&array, &oid)); + } else if (skip_prefix(line.buf, "contains_nondestructive ", &arg)) { + if (get_oid_hex(arg, &oid)) + die("not a hexadecimal SHA1: %s", arg); + printf("%d\n", oid_array_contains_nondestructive(&array, &oid)); } else if (!strcmp(line.buf, "clear")) oid_array_clear(&array); else if (!strcmp(line.buf, "for_each_unique")) oid_array_for_each_unique(&array, print_oid, NULL); + else if (!strcmp(line.buf, "for_each")) + oid_array_for_each(&array, print_oid, NULL); else die("unknown command: %s", line.buf); } diff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh index 5dda570b9a..05512b0461 100755 --- a/t/t0064-sha1-array.sh +++ b/t/t0064-sha1-array.sh @@ -32,6 +32,28 @@ test_expect_success 'ordered enumeration with duplicate suppression' ' test_cmp expect actual ' +test_expect_success 'contains_nondestructive finds existing' ' + echo 1 > expect && + echoid "" 88 44 aa 55 >> expect && + { + echoid append 88 44 aa 55 && + echoid contains_nondestructive 55 && + echo for_each + } | test-tool sha1-array >actual && + test_cmp expect actual +' + +test_expect_success 'contains_nondestructive non-existing query' ' + echo 0 > expect && + echoid "" 88 44 aa 55 >> expect && + { + echoid append 88 44 aa 55 && + echoid contains_nondestructive 33 && + echo for_each + } | test-tool sha1-array >actual && + test_cmp expect actual +' + test_expect_success 'lookup' ' { echoid append 88 44 aa 55 &&
From: Stefan Xenos <sxenos@gmail.com> Implement a "contains_nondestructive" function for oid_array that won't sort the array if it is unsorted. This can be used to test containment in the rare situations where the array order matters. The function has intentionally been given a name that is more cumbersome than the "lookup" function, which is what most callers will will want in most situations. --- sha1-array.c | 15 +++++++++++++++ sha1-array.h | 2 ++ t/helper/test-sha1-array.c | 6 ++++++ t/t0064-sha1-array.sh | 22 ++++++++++++++++++++++ 4 files changed, 45 insertions(+)