On Thu, May 19, 2011 at 6:34 PM, Jeff King <peff@xxxxxxxx> wrote:
> This is a generally useful abstraction, so let's let others
> make use of it. ÂThe refactoring is more or less a straight
> copy; however, functions and struct members have had their
> names changed to match string_list, which is the most
> similar data structure.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> I was tempted also to rename it to sha1_list to match string_list. But
> after working with commit_list recently, where the linked list nature
> was important, I began to think that string_list is perhaps mis-named.
>
> ÂMakefile   |  Â2 +
> Âbisect.c   |  70 ++++++++++++---------------------------------------------
> Âsha1-array.c | Â 43 +++++++++++++++++++++++++++++++++++
> Âsha1-array.h | Â 18 +++++++++++++++
> Â4 files changed, 78 insertions(+), 55 deletions(-)
> Âcreate mode 100644 sha1-array.c
> Âcreate mode 100644 sha1-array.h
>
> diff --git a/Makefile b/Makefile
> index 5379aaa..a357d58 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -540,6 +540,7 @@ LIB_H += rerere.h
> ÂLIB_H += resolve-undo.h
> ÂLIB_H += revision.h
> ÂLIB_H += run-command.h
> +LIB_H += sha1-array.h
> ÂLIB_H += sha1-lookup.h
> ÂLIB_H += sideband.h
> ÂLIB_H += sigchain.h
> @@ -642,6 +643,7 @@ LIB_OBJS += revision.o
> ÂLIB_OBJS += run-command.o
> ÂLIB_OBJS += server-info.o
> ÂLIB_OBJS += setup.o
> +LIB_OBJS += sha1-array.o
> ÂLIB_OBJS += sha1-lookup.o
> ÂLIB_OBJS += sha1_file.o
> ÂLIB_OBJS += sha1_name.o
> diff --git a/bisect.c b/bisect.c
> index 060c042..dd7e8ed 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -9,13 +9,7 @@
> Â#include "run-command.h"
> Â#include "log-tree.h"
> Â#include "bisect.h"
> -
> -struct sha1_array {
> - Â Â Â unsigned char (*sha1)[20];
> - Â Â Â int sha1_nr;
> - Â Â Â int sha1_alloc;
> - Â Â Â int sorted;
> -};
> +#include "sha1-array.h"
>
> Âstatic struct sha1_array good_revs;
> Âstatic struct sha1_array skipped_revs;
> @@ -425,22 +419,15 @@ static void argv_array_push_sha1(struct argv_array *array,
> Â Â Â Âargv_array_push(array, strbuf_detach(&buf, NULL));
> Â}
>
> -static void sha1_array_push(struct sha1_array *array,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â const unsigned char *sha1)
> -{
> - Â Â Â ALLOC_GROW(array->sha1, array->sha1_nr + 1, array->sha1_alloc);
> - Â Â Â hashcpy(array->sha1[array->sha1_nr++], sha1);
> -}
> -
> Âstatic int register_ref(const char *refname, const unsigned char *sha1,
> Â Â Â Â Â Â Â Â Â Â Â Âint flags, void *cb_data)
> Â{
> Â Â Â Âif (!strcmp(refname, "bad")) {
> Â Â Â Â Â Â Â Âcurrent_bad_sha1 = sha1;
> Â Â Â Â} else if (!prefixcmp(refname, "good-")) {
> - Â Â Â Â Â Â Â sha1_array_push(&good_revs, sha1);
> + Â Â Â Â Â Â Â sha1_array_append(&good_revs, sha1);
> Â Â Â Â} else if (!prefixcmp(refname, "skip-")) {
> - Â Â Â Â Â Â Â sha1_array_push(&skipped_revs, sha1);
> + Â Â Â Â Â Â Â sha1_array_append(&skipped_revs, sha1);
> Â Â Â Â}
>
> Â Â Â Âreturn 0;
> @@ -477,41 +464,14 @@ static void read_bisect_paths(struct argv_array *array)
> Â Â Â Âfclose(fp);
> Â}
>
> -static int array_cmp(const void *a, const void *b)
> -{
> - Â Â Â return hashcmp(a, b);
> -}
> -
> -static void sort_sha1_array(struct sha1_array *array)
> -{
> - Â Â Â qsort(array->sha1, array->sha1_nr, sizeof(*array->sha1), array_cmp);
> -
> - Â Â Â array->sorted = 1;
> -}
> -
> -static const unsigned char *sha1_access(size_t index, void *table)
> -{
> - Â Â Â unsigned char (*array)[20] = table;
> - Â Â Â return array[index];
> -}
> -
> -static int lookup_sha1_array(struct sha1_array *array,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst unsigned char *sha1)
> -{
> - Â Â Â if (!array->sorted)
> - Â Â Â Â Â Â Â sort_sha1_array(array);
> -
> - Â Â Â return sha1_pos(sha1, array->sha1, array->sha1_nr, sha1_access);
> -}
> -
> Âstatic char *join_sha1_array_hex(struct sha1_array *array, char delim)
> Â{
> Â Â Â Âstruct strbuf joined_hexs = STRBUF_INIT;
> Â Â Â Âint i;
>
> - Â Â Â for (i = 0; i < array->sha1_nr; i++) {
> + Â Â Â for (i = 0; i < array->nr; i++) {
> Â Â Â Â Â Â Â Âstrbuf_addstr(&joined_hexs, sha1_to_hex(array->sha1[i]));
> - Â Â Â Â Â Â Â if (i + 1 < array->sha1_nr)
> + Â Â Â Â Â Â Â if (i + 1 < array->nr)
> Â Â Â Â Â Â Â Â Â Â Â Âstrbuf_addch(&joined_hexs, delim);
> Â Â Â Â}
>
> @@ -546,13 +506,13 @@ struct commit_list *filter_skipped(struct commit_list *list,
> Â Â Â Âif (count)
> Â Â Â Â Â Â Â Â*count = 0;
>
> - Â Â Â if (!skipped_revs.sha1_nr)
> + Â Â Â if (!skipped_revs.nr)
> Â Â Â Â Â Â Â Âreturn list;
>
> Â Â Â Âwhile (list) {
> Â Â Â Â Â Â Â Âstruct commit_list *next = list->next;
> Â Â Â Â Â Â Â Âlist->next = NULL;
> - Â Â Â Â Â Â Â if (0 <= lookup_sha1_array(&skipped_revs,
> + Â Â Â Â Â Â Â if (0 <= sha1_array_lookup(&skipped_revs,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â list->item->object.sha1)) {
> Â Â Â Â Â Â Â Â Â Â Â Âif (skipped_first && !*skipped_first)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â*skipped_first = 1;
> @@ -647,7 +607,7 @@ static struct commit_list *managed_skipped(struct commit_list *list,
>
> Â Â Â Â*tried = NULL;
>
> - Â Â Â if (!skipped_revs.sha1_nr)
> + Â Â Â if (!skipped_revs.nr)
> Â Â Â Â Â Â Â Âreturn list;
>
> Â Â Â Âlist = filter_skipped(list, tried, 0, &count, &skipped_first);
> @@ -672,7 +632,7 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix,
> Â Â Â Â/* rev_argv.argv[0] will be ignored by setup_revisions */
> Â Â Â Âargv_array_push(&rev_argv, xstrdup("bisect_rev_setup"));
> Â Â Â Âargv_array_push_sha1(&rev_argv, current_bad_sha1, bad_format);
> - Â Â Â for (i = 0; i < good_revs.sha1_nr; i++)
> + Â Â Â for (i = 0; i < good_revs.nr; i++)
> Â Â Â Â Â Â Â Âargv_array_push_sha1(&rev_argv, good_revs.sha1[i],
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â good_format);
> Â Â Â Âargv_array_push(&rev_argv, xstrdup("--"));
> @@ -772,12 +732,12 @@ static struct commit *get_commit_reference(const unsigned char *sha1)
>
> Âstatic struct commit **get_bad_and_good_commits(int *rev_nr)
> Â{
> - Â Â Â int len = 1 + good_revs.sha1_nr;
> + Â Â Â int len = 1 + good_revs.nr;
> Â Â Â Âstruct commit **rev = xmalloc(len * sizeof(*rev));
> Â Â Â Âint i, n = 0;
>
> Â Â Â Ârev[n++] = get_commit_reference(current_bad_sha1);
> - Â Â Â for (i = 0; i < good_revs.sha1_nr; i++)
> + Â Â Â for (i = 0; i < good_revs.nr; i++)
> Â Â Â Â Â Â Â Ârev[n++] = get_commit_reference(good_revs.sha1[i]);
> Â Â Â Â*rev_nr = n;
>
> @@ -840,9 +800,9 @@ static void check_merge_bases(void)
> Â Â Â Â Â Â Â Âconst unsigned char *mb = result->item->object.sha1;
> Â Â Â Â Â Â Â Âif (!hashcmp(mb, current_bad_sha1)) {
> Â Â Â Â Â Â Â Â Â Â Â Âhandle_bad_merge_base();
> - Â Â Â Â Â Â Â } else if (0 <= lookup_sha1_array(&good_revs, mb)) {
> + Â Â Â Â Â Â Â } else if (0 <= sha1_array_lookup(&good_revs, mb)) {
> Â Â Â Â Â Â Â Â Â Â Â Âcontinue;
> - Â Â Â Â Â Â Â } else if (0 <= lookup_sha1_array(&skipped_revs, mb)) {
> + Â Â Â Â Â Â Â } else if (0 <= sha1_array_lookup(&skipped_revs, mb)) {
> Â Â Â Â Â Â Â Â Â Â Â Âhandle_skipped_merge_base(mb);
> Â Â Â Â Â Â Â Â} else {
> Â Â Â Â Â Â Â Â Â Â Â Âprintf("Bisecting: a merge base must be tested\n");
> @@ -903,7 +863,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix)
> Â Â Â Â Â Â Â Âreturn;
>
> Â Â Â Â/* Bisecting with no good rev is ok. */
> - Â Â Â if (good_revs.sha1_nr == 0)
> + Â Â Â if (good_revs.nr == 0)
> Â Â Â Â Â Â Â Âreturn;
>
> Â Â Â Â/* Check if all good revs are ancestor of the bad rev. */
> @@ -968,7 +928,7 @@ int bisect_next_all(const char *prefix)
> Â Â Â Âbisect_common(&revs);
>
> Â Â Â Ârevs.commits = find_bisection(revs.commits, &reaches, &all,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â!!skipped_revs.sha1_nr);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â!!skipped_revs.nr);
> Â Â Â Ârevs.commits = managed_skipped(revs.commits, &tried);
>
> Â Â Â Âif (!revs.commits) {
> diff --git a/sha1-array.c b/sha1-array.c
> new file mode 100644
> index 0000000..5b75a5a
> --- /dev/null
> +++ b/sha1-array.c
> @@ -0,0 +1,43 @@
> +#include "cache.h"
> +#include "sha1-array.h"
> +#include "sha1-lookup.h"
> +
> +void sha1_array_append(struct sha1_array *array, const unsigned char *sha1)
> +{
> + Â Â Â ALLOC_GROW(array->sha1, array->nr + 1, array->alloc);
> + Â Â Â hashcpy(array->sha1[array->nr++], sha1);
> + Â Â Â array->sorted = 0;
> +}
> +
> +static int void_hashcmp(const void *a, const void *b)
> +{
> + Â Â Â return hashcmp(a, b);
> +}
> +
> +void sha1_array_sort(struct sha1_array *array)
> +{
> + Â Â Â qsort(array->sha1, array->nr, sizeof(*array->sha1), void_hashcmp);
> + Â Â Â array->sorted = 1;
> +}
> +
> +static const unsigned char *sha1_access(size_t index, void *table)
> +{
> + Â Â Â unsigned char (*array)[20] = table;
> + Â Â Â return array[index];
> +}
> +
> +int sha1_array_lookup(struct sha1_array *array, const unsigned char *sha1)
> +{
> + Â Â Â if (!array->sorted)
> + Â Â Â Â Â Â Â sha1_array_sort(array);
> + Â Â Â return sha1_pos(sha1, array->sha1, array->nr, sha1_access);
> +}
> +
> +void sha1_array_clear(struct sha1_array *array)
> +{
> + Â Â Â free(array->sha1);
> + Â Â Â array->sha1 = NULL;
> + Â Â Â array->nr = 0;
> + Â Â Â array->alloc = 0;
> + Â Â Â array->sorted = 0;
> +}
> diff --git a/sha1-array.h b/sha1-array.h
> new file mode 100644
> index 0000000..15d3b6b
> --- /dev/null
> +++ b/sha1-array.h
> @@ -0,0 +1,18 @@
> +#ifndef SHA1_ARRAY_H
> +#define SHA1_ARRAY_H
> +
> +struct sha1_array {
> + Â Â Â unsigned char (*sha1)[20];
> + Â Â Â int nr;
> + Â Â Â int alloc;
Would be worth to change from int to unsigned int? Maybe is fine as is
though. It's that in some places we use unsigned int (string_list is
one example).
> + Â Â Â int sorted;
> +};
> +
> +#define SHA1_ARRAY_INIT { NULL, 0, 0, 0 }
> +
> +void sha1_array_append(struct sha1_array *array, const unsigned char *sha1);
> +void sha1_array_sort(struct sha1_array *array);
> +int sha1_array_lookup(struct sha1_array *array, const unsigned char *sha1);
> +void sha1_array_clear(struct sha1_array *array);
> +
Thanks for the nice API!
> +#endif /* SHA1_ARRAY_H */
> --
> 1.7.5.8.ga95ab
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html