Re: [PATCH 05/19] reftable/record: stop using `BUG()` in `reftable_record_init()`
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Jan 27, 2025, 17:36 UTC
- Message-ID
- <v3rkbym7a6tm3co6okuleppiabx6t4zxbcn5iei66iv3dxqkrf@wmqsctl2mmgn>
- In-Reply-To
- <20250127-pks-reftable-drop-git-compat-util-v1-5-6e280a564877@pks.im>
On 25/01/27 02:04PM, Patrick Steinhardt wrote:
Show 9 quoted lines
> We're aborting the program via `BUG()` in case `reftable_record_init()` > was invoked with an unknown record type. This is bad because we may now > die in library code, and because it makes us depend on the Git codebase. > > Refactor the code such that `reftable_record_init()` can return an error > code to the caller. Adapt any callers accordingly. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > ---
[snip]
Show 26 quoted lines
> diff --git a/reftable/record.c b/reftable/record.c
> index d1664c47ca..31985bb977 100644
> --- a/reftable/record.c
> +++ b/reftable/record.c
> @@ -1301,7 +1301,7 @@ reftable_record_vtable(struct reftable_record *rec)
> abort();
> }
>
> -void reftable_record_init(struct reftable_record *rec, uint8_t typ)
> +int reftable_record_init(struct reftable_record *rec, uint8_t typ)
> {
> memset(rec, 0, sizeof(*rec));
> rec->type = typ;
> @@ -1310,11 +1310,11 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ)
> case BLOCK_TYPE_REF:
> case BLOCK_TYPE_LOG:
> case BLOCK_TYPE_OBJ:
> - return;
> + return 0;
> case BLOCK_TYPE_INDEX:
> reftable_buf_init(&rec->u.idx.last_key);
> - return;
> + return 0;
> default:
> - BUG("unhandled record type");
> + return REFTABLE_API_ERROR;I was initially unsure if `REFTABLE_API_ERROR` would be the most appropriate error to return here in this situation, but looking at its documented use case, I would say this fits as a "misuse of the API". The other option would be to add a more granular error type to indicate the unsupported record type, but that seems unnecessary here.
> } > }