linux-kernel - Re: [PATCH v2 3/5] Add kunit tests for iio_divide_by

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49b24a0d-a2ac-4620-9a1e-a94e5a2db075@baylibre.com>
Date: Thu, 25 Sep 2025 15:26:38 -0500
From: David Lechner <dlechner@...libre.com>
To: Romain Gantois <romain.gantois@...tlin.com>,
 Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Jonathan Cameron <jic23@...nel.org>,
 Nuno Sá <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>
Cc: Hans de Goede <hansg@...nel.org>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-iio@...r.kernel.org
Subject: Re: [PATCH v2 3/5] Add kunit tests for iio_divide_by_value()

On 9/25/25 7:37 AM, Romain Gantois wrote:

...

> +static void __iio_test_iio_divide_by_integer(struct kunit *test, s64 numerator)
> +{
> +	int ret, result, val;
> +
> +	val = 42;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator, val));
> +
> +	val = -23;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator, val));
> +
> +	val = 0;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, -ERANGE);

I would expect EDOM for divide by 0 rather than ERANGE. The function is
undefined at that point.

> +}
> +
> +static void iio_test_iio_divide_by_integer(struct kunit *test)
> +{
> +	__iio_test_iio_divide_by_integer(test, 2000);
> +	__iio_test_iio_divide_by_integer(test, -2000);
> +}
> +
> +static void __iio_test_iio_divide_by_fixedpoint(struct kunit *test, s64 numerator)
> +{
> +	int ret, result, val, val2;
> +
> +	/* positive >= 1 (1.5) */
> +	val = 1;
> +	val2 = 500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 15));
> +
> +	val = 1;
> +	val2 = 500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 15));
> +
> +	/* positive < 1 (0.5) */
> +	val = 0;
> +	val2 = 500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 5));
> +
> +	val = 0;
> +	val2 = 500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * 10, 5));
> +
> +	/* negative <= -1 (-1.5) */
> +	val = -1;
> +	val2 = 500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 15));
> +
> +	val = -1;
> +	val2 = 500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 15));
> +
> +	/* negative > -1 (-0.5) */
> +	val = 0;
> +	val2 = -500000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_MICRO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 5));
> +
> +	val = 0;
> +	val2 = -500000000;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_INT_PLUS_NANO, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64(numerator * -10, 5));
> +
> +	/* Zero */
> +	val = 0;

Odd to break the pattern an not have `val2 = 0;` here.

> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, 0);
> +	KUNIT_EXPECT_EQ(test, ret, -ERANGE);
> +}

...

> +static void __iio_test_iio_divide_by_fractional_log2(struct kunit *test, s64 numerator)
> +{
> +	int ret, result, val, val2;
> +
> +	/* positive < 1 (123/1024) */
> +	val = 123;
> +	val2 = 10;
> +	ret = iio_divide_by_value(&result, numerator, IIO_VAL_FRACTIONAL_LOG2, val, val2);
> +	KUNIT_EXPECT_EQ(test, ret, IIO_VAL_INT);
> +	KUNIT_EXPECT_EQ(test, result, div_s64((numerator << val2), val));

My instinct would be to write it like this:

	div_s64((numerator * 1024), 123)

This follows how it was done in __iio_test_iio_divide_by_fixedpoint() and
makes it easier to see that it matchs exactly the value in the comment.