Re: geom_journal panic: wrong offset 500107860990 for sectorsize 512 - RESOLVED

On Mon, Apr 02, 2007 at 11:56:24PM -0500, Eric Anderson wrote:
> Here is a patch that adds that functionality.  Can be found here:
> 
> http://www.googlebit.com/freebsd/patches/gjournal_size_expression.patch
> 
> and attached.

Thanks for the patch. I'd prefer to have such functionality as a part of
libutil. Simlar to humanize_number(3), but the other way around.
Some comments below.

> + * Convert an expression of the following forms to a uintmax_t.
> + * 	1) A positive decimal number.
> + *	2) A positive decimal number followed by a 'b' or 'B' (mult by 512).

Why? If I give '-s 1024B' it means I want 1024 bytes, not 1024*512
bytes. I would multiply by 512 if I receive number of sectors or
something. My suggestion is to accept 'b' and 'B', but ignore it (or
multiply by 1).

> + *	3) A positive decimal number followed by a 'k' or 'K' (mult by 1 << 10).
> + *	4) A positive decimal number followed by a 'm' or 'M' (mult by 1 << 20).
> + *	5) A positive decimal number followed by a 'g' or 'G' (mult by 1 << 30).

I'd add 't' and 'T' as well.

> + *	5) A positive decimal number followed by a 'w' or 'W' (mult by sizeof int).

I suggest dropping it, I don't really see a use for it...

> + */
> +intmax_t
> +gctl_get_numexpr(struct gctl_req *req, const char *pfmt, ...)
> +{
> +	const char *val;
> +	va_list ap;
> +	uintmax_t num, mult, prevnum;
> +	char *expr;
> +
> +	va_start(ap, pfmt);
> +	val = gctl_get_param(req, 0, pfmt, ap);
> +
> +	num = strtouq(val, &expr, 0);
> +
> +	if (expr == val)			/* No valid digits. */
> +		printf("illegal numeric value\n");
> +
> +	mult = 0;
> +	switch (*expr) {
> +	case 'B':
> +	case 'b':
> +		mult = 512;
> +		break;
> +	case 'K':
> +	case 'k':
> +		mult = 1 << 10;
> +		break;
> +	case 'M':
> +	case 'm':
> +		mult = 1 << 20;
> +		break;
> +	case 'G':
> +	case 'g':
> +		mult = 1 << 30;
> +		break;
> +	case 'W':
> +	case 'w':
> +		mult = sizeof(int);
> +		break;
> +	default:
> +		;
> +	}
> +
> +	if (mult != 0) {

Maybe just set mult to 1 by default and drop this check?

> +		prevnum = num;
> +		num *= mult;
> +		/* Check for overflow. */
> +		if (num / mult != prevnum)
> +			printf("overflow in argument\n");
> +		expr++;
> +	}

-- 
Pawel Jakub Dawidek                       http://www.wheel.pl
pjd_at_FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!