On 25/01/27 02:04PM, Patrick Steinhardt wrote:
> 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@xxxxxx>
> ---
[snip]
> 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.
> }
> }
