From: Karthik Nayak <karthik.188@gmail.com>
Date: Wed, 24 Sep 2025 18:44:06 GMT
Subject: Re: [PATCH v3 8/8] refs/reftable: add fsck check for checking the table name
Message-ID: <CAOLa=ZS+CGh6kuT87xoR_GnxJkBABk3g0CtOqU9wfnCVS=AcBw@mail.gmail.com>
In-Reply-To: <aNOHr7lm4WXUyHa5@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

>> +static int reftable_fsck_error_handler(struct reftable_fsck_info *info,
>> +				       void *cb_data)
>> +{
>> +	struct fsck_ref_report report = { .path = info->path };
>> +	struct fsck_options *o = cb_data;
>> +	enum fsck_msg_id msg_id;
>> +
>> +	if (info->error < 0 || info->error >= REFTABLE_FSCK_MAX_VALUE)
>> +		BUG("unknown fsck error: %d", info->error);
>
> `info->error` is an enum, and whether or not it is signed is an
> implementation detail of the platform. But I wonder whether this check
> may cause some platforms to warn about an impossible condition.
>

I didn't really think of that. I guess typecasting it to an int would be
the best way forward here.

>> +
>> +	msg_id = fsck_msg_id_map[info->error];
>> +
>> +	if (!msg_id)
>> +		BUG("fsck_msg_id value missing for reftable error: %d", info->error);
>
> Yup, makes sense.
>
> Patrick

Thanks for the review.