| To: | "Christoph Hellwig" <hch@xxxxxxxxxxxxx>, "Eric Sandeen" <sandeen@xxxxxxxxxxx> |
|---|---|
| Subject: | RE: [PATCH 01/14] repair: merge scanfunc_bno and scanfunc_cnt |
| From: | "Alex Elder" <aelder@xxxxxxx> |
| Date: | Tue, 13 Oct 2009 18:36:47 -0500 |
| Cc: | <xfs@xxxxxxxxxxx> |
| In-reply-to: | <20091013221335.GA30832@xxxxxxxxxxxxx> |
| Thread-index: | AcpMViglxjfAl2d8Sb28xGHk/JuLYgAB5WSQ |
| Thread-topic: | [PATCH 01/14] repair: merge scanfunc_bno and scanfunc_cnt |
On , Christoph Hellwig wrote:
> On Mon, Oct 12, 2009 at 11:53:00AM -0500, Eric Sandeen wrote:
>>
>> Should we explicitly test that this is either XFS_ABTC_MAGIC or
>> XFS_ABTB_MAGIC here to avoid any programming-error
>> type problems?
>
> We really only have two freespace btrees. But I'll add an assert
> just to be sure.
>
>>> - else {
>>> + break;
>>> + case XR_E_FREE1:
>>> + /*
>>> + * no warning messages -- we'll catch
>>> + * FREE1 blocks later
>>> + */
>>> + if (magic != XFS_ABTB_MAGIC) {
>>
>> Why not make this explicitly "if (magic == XFS_ABTC_MAGIC)" - I guess it
>> seems potentially
>> more future-proof to me though I don't suppose we'll ever get a new type
>> here. :)
>> The positive test seems clearer to me but *shrug*.
>
> Ok, changed.
With changes as described, looks good.
Reviewed-by: Alex Elder <aelder@xxxxxxx>
|
| <Prev in Thread] | Current Thread | [Next in Thread> |
|---|---|---|
| ||
| Previous by Date: | Re: Wrapped journal record corruption on read at recovery - patch attached (was Re: XFS corruption with failover), Christoph Hellwig |
|---|---|
| Next by Date: | RE: [PATCH 02/14] repair: reduce byte swap operations inscanfunc_allocbt, Alex Elder |
| Previous by Thread: | Re: [PATCH 01/14] repair: merge scanfunc_bno and scanfunc_cnt, Christoph Hellwig |
| Next by Thread: | Re: [PATCH 02/14] repair: reduce byte swap operations in scanfunc_allocbt, Eric Sandeen |
| Indexes: | [Date] [Thread] [Top] [All Lists] |