On Tue, Feb 28, 2012 at 12:42:24PM -0500, Christoph Hellwig wrote:
> It looks like we currently never grow the variable-width nlink array
> if only the on-disk nlink size overflows 8 bits. This leads to a major
> mess in nlink counting, and eventually an assert in phase7.
>
> Replace the indirect all mess with a union that allows doing proper
> array arithmetics while we're at it.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks fairly sane to me. I can't see any obvious problems with the
change, though it might be worth adding an overflow warning to the
code:
> + switch (irec->nlink_size) {
> + case sizeof(__uint8_t):
> + if (irec->ino_un.ex_data->counted_nlinks.un8[ino_offset] < 0xff) {
> + irec->ino_un.ex_data->counted_nlinks.un8[ino_offset]++;
> + break;
> + }
> nlink_grow_8_to_16(irec);
> - disk_nlink_16_set(irec, ino_offset, nlinks);
> - } else
> - irec->disk_nlinks[ino_offset] = nlinks;
> + /*FALLTHRU*/
> + case sizeof(__uint16_t):
> + if (irec->ino_un.ex_data->counted_nlinks.un16[ino_offset] < 0xffff) {
> + irec->ino_un.ex_data->counted_nlinks.un16[ino_offset]++;
> + break;
> + }
> + nlink_grow_16_to_32(irec);
> + /*FALLTHRU*/
> + case sizeof(__uint32_t):
> + irec->ino_un.ex_data->counted_nlinks.un32[ino_offset]++;
To check for UINT_MAX before the increment....
> + switch (irec->nlink_size) {
> + case sizeof(__uint8_t):
> + ASSERT(irec->ino_un.ex_data->counted_nlinks.un8[ino_offset] > 0);
> + refs = --irec->ino_un.ex_data->counted_nlinks.un8[ino_offset];
> + break;
> + case sizeof(__uint16_t):
> + ASSERT(irec->ino_un.ex_data->counted_nlinks.un16[ino_offset] > 0);
> + refs = --irec->ino_un.ex_data->counted_nlinks.un16[ino_offset];
> + break;
> + case sizeof(__uint32_t):
> + ASSERT(irec->ino_un.ex_data->counted_nlinks.un32[ino_offset] > 0);
> + refs = --irec->ino_un.ex_data->counted_nlinks.un32[ino_offset];
> + break;
> + default:
> + ASSERT(0);
Similar to the underflow checks here.
Otherwise:
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
