diff mbox series

[v5,13/16] reftable: remove outdated file reftable.c

Message ID 4175089ec432da921d158fec9ccb1902be710af6.1640199396.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 9391b88dab5145f01c9b8c3d79dba567058086e1
Headers show
Series Reftable coverity fixes | expand

Commit Message

Han-Wen Nienhuys Dec. 22, 2021, 6:56 p.m. UTC
From: Han-Wen Nienhuys <hanwen@google.com>

This was renamed to generic.c, but the origin was never removed

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 reftable/reftable.c | 115 --------------------------------------------
 1 file changed, 115 deletions(-)
 delete mode 100644 reftable/reftable.c

Comments

Junio C Hamano Dec. 22, 2021, 10:51 p.m. UTC | #1
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> This was renamed to generic.c, but the origin was never removed
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  reftable/reftable.c | 115 --------------------------------------------
>  1 file changed, 115 deletions(-)
>  delete mode 100644 reftable/reftable.c

That's embarrassing for all reviewers of past reftable patches.
Ævar Arnfjörð Bjarmason Dec. 24, 2021, 4:53 p.m. UTC | #2
On Wed, Dec 22 2021, Junio C Hamano wrote:

> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Han-Wen Nienhuys <hanwen@google.com>
>>
>> This was renamed to generic.c, but the origin was never removed
>>
>> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>> ---
>>  reftable/reftable.c | 115 --------------------------------------------
>>  1 file changed, 115 deletions(-)
>>  delete mode 100644 reftable/reftable.c
>
> That's embarrassing for all reviewers of past reftable patches.

As one of those reviewers: I think it suggests something different:

Various people looked it over, but this amount of code is just too much
for someone to review in one sitting, so it's not unexpected that
various things like this slipped through.

The current landing of this topic on "master" is based on a plan I
suggested, i.e. to have the library and its own tests in "master" first,
and not to do any of the integration yet.

Exactly because we expected that we would not be getting any of this
right the first time around.

So if anything I think these current topics and recent reftable activity
suggest that we should have been more willing to get it into "master"
earlier, and not wait until all bugs in it were solved beforehand.

I.e. there's bugs in the code, but they're well-contained, since nothing
in-tree uses it except its own tests.

And by having merged it down we're benefiting from more eyes, both in
terms of human review, and in the review of various tooling (like
coverity) that's being run on "master" but not on some random branch
that's languishing in "seen" for months.
diff mbox series

Patch

diff --git a/reftable/reftable.c b/reftable/reftable.c
deleted file mode 100644
index 0e4607a7cd6..00000000000
--- a/reftable/reftable.c
+++ /dev/null
@@ -1,115 +0,0 @@ 
-/*
-Copyright 2020 Google LLC
-
-Use of this source code is governed by a BSD-style
-license that can be found in the LICENSE file or at
-https://developers.google.com/open-source/licenses/bsd
-*/
-
-#include "basics.h"
-#include "record.h"
-#include "generic.h"
-#include "reftable-iterator.h"
-#include "reftable-generic.h"
-
-int reftable_table_seek_ref(struct reftable_table *tab,
-			    struct reftable_iterator *it, const char *name)
-{
-	struct reftable_ref_record ref = {
-		.refname = (char *)name,
-	};
-	struct reftable_record rec = { NULL };
-	reftable_record_from_ref(&rec, &ref);
-	return tab->ops->seek_record(tab->table_arg, it, &rec);
-}
-
-int reftable_table_read_ref(struct reftable_table *tab, const char *name,
-			    struct reftable_ref_record *ref)
-{
-	struct reftable_iterator it = { NULL };
-	int err = reftable_table_seek_ref(tab, &it, name);
-	if (err)
-		goto done;
-
-	err = reftable_iterator_next_ref(&it, ref);
-	if (err)
-		goto done;
-
-	if (strcmp(ref->refname, name) ||
-	    reftable_ref_record_is_deletion(ref)) {
-		reftable_ref_record_release(ref);
-		err = 1;
-		goto done;
-	}
-
-done:
-	reftable_iterator_destroy(&it);
-	return err;
-}
-
-uint64_t reftable_table_max_update_index(struct reftable_table *tab)
-{
-	return tab->ops->max_update_index(tab->table_arg);
-}
-
-uint64_t reftable_table_min_update_index(struct reftable_table *tab)
-{
-	return tab->ops->min_update_index(tab->table_arg);
-}
-
-uint32_t reftable_table_hash_id(struct reftable_table *tab)
-{
-	return tab->ops->hash_id(tab->table_arg);
-}
-
-void reftable_iterator_destroy(struct reftable_iterator *it)
-{
-	if (!it->ops) {
-		return;
-	}
-	it->ops->close(it->iter_arg);
-	it->ops = NULL;
-	FREE_AND_NULL(it->iter_arg);
-}
-
-int reftable_iterator_next_ref(struct reftable_iterator *it,
-			       struct reftable_ref_record *ref)
-{
-	struct reftable_record rec = { NULL };
-	reftable_record_from_ref(&rec, ref);
-	return iterator_next(it, &rec);
-}
-
-int reftable_iterator_next_log(struct reftable_iterator *it,
-			       struct reftable_log_record *log)
-{
-	struct reftable_record rec = { NULL };
-	reftable_record_from_log(&rec, log);
-	return iterator_next(it, &rec);
-}
-
-int iterator_next(struct reftable_iterator *it, struct reftable_record *rec)
-{
-	return it->ops->next(it->iter_arg, rec);
-}
-
-static int empty_iterator_next(void *arg, struct reftable_record *rec)
-{
-	return 1;
-}
-
-static void empty_iterator_close(void *arg)
-{
-}
-
-static struct reftable_iterator_vtable empty_vtable = {
-	.next = &empty_iterator_next,
-	.close = &empty_iterator_close,
-};
-
-void iterator_set_empty(struct reftable_iterator *it)
-{
-	assert(!it->ops);
-	it->iter_arg = NULL;
-	it->ops = &empty_vtable;
-}