[PATCH 1/2] repair: fix the variable-width nlink array

[Dave Chinner]

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 at lst.de>

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 at redhat.com>

-- 
Dave Chinner
david at fromorbit.com