Re: [PATCH 1/2] Fix sign extension of bitmap_offset in super1.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/26/12 17:18, Doug Ledford wrote:
> On 04/26/2012 11:12 AM, Jes.Sorensen@xxxxxxxxxx wrote:
>> > From: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
>> > 
>> > fbdef49811c9e2b54e2064d9af68cfffa77c6e77 incorrectly tried to fix sign
>> > extension of the bitmap offset. However mdinfo->bitmap_offset is a u32
>> > and needs to be converted to a 32 bit signed integer before the sign
>> > extension.
>> > 
>> > Signed-off-by: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
> I was scratching my head over this patch, saying to myself "But won't
> that cause us to truncate large values of bitmap_offset?"  And it will,
> but I see your point now, that's *exactly* the problem if we don't do
> the sign conversion before the extension, the actual bitmap_offset
> should really be signed in order to support negative offsets, but since
> it isn't, when we save a negative offset into bitmap_offset it appears
> as a really large positive offset, and then when we sign extend to long,
> it keeps the large size positive offset instead of picking up the
> negative offset.  Gotcha.  So, I see why this works, but do you think it
> should be fixed this way, or by converting bitmap_offset to type int32
> instead of uint32?

Heh, I have to admit I cheated too and asked Richard Henderson for help
as I couldn't figure out why the sign conversion failed. Otherwise I
would probably still have been scratching my head over it :)

I noticed other parts of the code already handled it this way, so my fix
is consistent with that, but we could do both. Neil?

Cheers,
Jes


--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux