Message ID | 8f69f4deac014f558bab186444bac2e8@AcuMS.aculab.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | minmax.h: Cleanups and minor optimisations | expand |
Hi David, kernel test robot noticed the following build warnings: [auto build test WARNING on next-20241121] url: https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-h-Add-whitespace-around-operators-and-after-commas/20241121-152617 base: next-20241121 patch link: https://lore.kernel.org/r/8f69f4deac014f558bab186444bac2e8%40AcuMS.aculab.com patch subject: [PATCH next 6/7] minmax.h: Simplify the variants of clamp() config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20241123/202411230458.dhZwh3TT-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241123/202411230458.dhZwh3TT-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411230458.dhZwh3TT-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from include/linux/kernel.h:28, from include/linux/cpumask.h:11, from arch/loongarch/include/asm/processor.h:9, from arch/loongarch/include/asm/thread_info.h:15, from include/linux/thread_info.h:60, from include/asm-generic/current.h:6, from ./arch/loongarch/include/generated/asm/current.h:1, from include/linux/sched.h:12, from include/linux/delay.h:13, from drivers/iio/magnetometer/yamaha-yas530.c:27: drivers/iio/magnetometer/yamaha-yas530.c: In function 'yas537_measure': >> include/linux/minmax.h:188:20: warning: overflow in conversion from 'long unsigned int' to 's32' {aka 'int'} changes value from '18446744073709543424' to '-8192' [-Woverflow] 188 | type ulo = (lo); \ | ^ include/linux/minmax.h:197:9: note: in expansion of macro '__clamp_once' 197 | __clamp_once(type, val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_)) | ^~~~~~~~~~~~ include/linux/minmax.h:233:32: note: in expansion of macro '__careful_clamp' 233 | #define clamp_val(val, lo, hi) __careful_clamp(typeof(val), val, lo, hi) | ^~~~~~~~~~~~~~~ drivers/iio/magnetometer/yamaha-yas530.c:414:25: note: in expansion of macro 'clamp_val' 414 | clamp_val(h[i], -BIT(13), BIT(13) - 1); | ^~~~~~~~~ vim +188 include/linux/minmax.h 172 173 /** 174 * min_not_zero - return the minimum that is _not_ zero, unless both are zero 175 * @x: value1 176 * @y: value2 177 */ 178 #define min_not_zero(x, y) ({ \ 179 typeof(x) __x = (x); \ 180 typeof(y) __y = (y); \ 181 __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); }) 182 183 #define __clamp(val, lo, hi) \ 184 ((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val))) 185 186 #define __clamp_once(type, val, lo, hi, uval, ulo, uhi) ({ \ 187 type uval = (val); \ > 188 type ulo = (lo); \ 189 type uhi = (hi); \ 190 BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \ 191 "clamp() low limit " #lo " greater than high limit " #hi); \ 192 BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi), \ 193 "clamp("#val", "#lo", "#hi") signedness error"); \ 194 __clamp(uval, ulo, uhi); }) 195
Hi David,
kernel test robot noticed the following build warnings:
[auto build test WARNING on next-20241121]
url: https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-h-Add-whitespace-around-operators-and-after-commas/20241121-152617
base: next-20241121
patch link: https://lore.kernel.org/r/8f69f4deac014f558bab186444bac2e8%40AcuMS.aculab.com
patch subject: [PATCH next 6/7] minmax.h: Simplify the variants of clamp()
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241128/202411282222.oF0B4110-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241128/202411282222.oF0B4110-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411282222.oF0B4110-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/iio/magnetometer/yamaha-yas530.c:30:
In file included from include/linux/i2c.h:13:
In file included from include/linux/acpi.h:14:
In file included from include/linux/device.h:32:
In file included from include/linux/device/driver.h:21:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/s390/include/asm/elf.h:181:
In file included from arch/s390/include/asm/mmu_context.h:11:
In file included from arch/s390/include/asm/pgalloc.h:18:
In file included from include/linux/mm.h:2223:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/magnetometer/yamaha-yas530.c:414:20: warning: implicit conversion from 'unsigned long' to 'typeof (h[i])' (aka 'int') changes value from 18446744073709543424 to -8192 [-Wconstant-conversion]
414 | clamp_val(h[i], -BIT(13), BIT(13) - 1);
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:233:66: note: expanded from macro 'clamp_val'
233 | #define clamp_val(val, lo, hi) __careful_clamp(typeof(val), val, lo, hi)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
include/linux/minmax.h:197:26: note: expanded from macro '__careful_clamp'
197 | __clamp_once(type, val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
| ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:188:14: note: expanded from macro '__clamp_once'
188 | type ulo = (lo); \
| ~~~ ^~
5 warnings generated.
vim +414 drivers/iio/magnetometer/yamaha-yas530.c
de8860b1ed4701 Linus Walleij 2020-12-24 358
65f79b50103067 Jakob Hauser 2022-08-13 359 /**
65f79b50103067 Jakob Hauser 2022-08-13 360 * yas537_measure() - Make a measure from the hardware
65f79b50103067 Jakob Hauser 2022-08-13 361 * @yas5xx: The device state
65f79b50103067 Jakob Hauser 2022-08-13 362 * @t: the raw temperature measurement
65f79b50103067 Jakob Hauser 2022-08-13 363 * @x: the raw x axis measurement
65f79b50103067 Jakob Hauser 2022-08-13 364 * @y1: the y1 axis measurement
65f79b50103067 Jakob Hauser 2022-08-13 365 * @y2: the y2 axis measurement
65f79b50103067 Jakob Hauser 2022-08-13 366 * @return: 0 on success or error code
65f79b50103067 Jakob Hauser 2022-08-13 367 */
65f79b50103067 Jakob Hauser 2022-08-13 368 static int yas537_measure(struct yas5xx *yas5xx, u16 *t, u16 *x, u16 *y1, u16 *y2)
de8860b1ed4701 Linus Walleij 2020-12-24 369 {
65f79b50103067 Jakob Hauser 2022-08-13 370 struct yas5xx_calibration *c = &yas5xx->calibration;
65f79b50103067 Jakob Hauser 2022-08-13 371 unsigned int busy;
65f79b50103067 Jakob Hauser 2022-08-13 372 u8 data[8];
65f79b50103067 Jakob Hauser 2022-08-13 373 u16 xy1y2[3];
65f79b50103067 Jakob Hauser 2022-08-13 374 s32 h[3], s[3];
65f79b50103067 Jakob Hauser 2022-08-13 375 int i, ret;
65f79b50103067 Jakob Hauser 2022-08-13 376
65f79b50103067 Jakob Hauser 2022-08-13 377 mutex_lock(&yas5xx->lock);
65f79b50103067 Jakob Hauser 2022-08-13 378
65f79b50103067 Jakob Hauser 2022-08-13 379 /* Contrary to YAS530/532, also a "cont" bit is set, meaning unknown */
65f79b50103067 Jakob Hauser 2022-08-13 380 ret = regmap_write(yas5xx->map, YAS537_MEASURE, YAS5XX_MEASURE_START |
65f79b50103067 Jakob Hauser 2022-08-13 381 YAS5XX_MEASURE_CONT);
65f79b50103067 Jakob Hauser 2022-08-13 382 if (ret < 0)
65f79b50103067 Jakob Hauser 2022-08-13 383 goto out_unlock;
65f79b50103067 Jakob Hauser 2022-08-13 384
65f79b50103067 Jakob Hauser 2022-08-13 385 /* Use same timeout like YAS530/532 but the bit is in data row 2 */
65f79b50103067 Jakob Hauser 2022-08-13 386 ret = regmap_read_poll_timeout(yas5xx->map, YAS5XX_MEASURE_DATA + 2, busy,
65f79b50103067 Jakob Hauser 2022-08-13 387 !(busy & YAS5XX_MEASURE_DATA_BUSY),
65f79b50103067 Jakob Hauser 2022-08-13 388 500, 20000);
65f79b50103067 Jakob Hauser 2022-08-13 389 if (ret) {
65f79b50103067 Jakob Hauser 2022-08-13 390 dev_err(yas5xx->dev, "timeout waiting for measurement\n");
65f79b50103067 Jakob Hauser 2022-08-13 391 goto out_unlock;
65f79b50103067 Jakob Hauser 2022-08-13 392 }
65f79b50103067 Jakob Hauser 2022-08-13 393
65f79b50103067 Jakob Hauser 2022-08-13 394 ret = regmap_bulk_read(yas5xx->map, YAS5XX_MEASURE_DATA,
65f79b50103067 Jakob Hauser 2022-08-13 395 data, sizeof(data));
65f79b50103067 Jakob Hauser 2022-08-13 396 if (ret)
65f79b50103067 Jakob Hauser 2022-08-13 397 goto out_unlock;
65f79b50103067 Jakob Hauser 2022-08-13 398
65f79b50103067 Jakob Hauser 2022-08-13 399 mutex_unlock(&yas5xx->lock);
65f79b50103067 Jakob Hauser 2022-08-13 400
65f79b50103067 Jakob Hauser 2022-08-13 401 *t = get_unaligned_be16(&data[0]);
65f79b50103067 Jakob Hauser 2022-08-13 402 xy1y2[0] = FIELD_GET(GENMASK(13, 0), get_unaligned_be16(&data[2]));
65f79b50103067 Jakob Hauser 2022-08-13 403 xy1y2[1] = get_unaligned_be16(&data[4]);
65f79b50103067 Jakob Hauser 2022-08-13 404 xy1y2[2] = get_unaligned_be16(&data[6]);
65f79b50103067 Jakob Hauser 2022-08-13 405
65f79b50103067 Jakob Hauser 2022-08-13 406 /* The second version of YAS537 needs to include calibration coefficients */
65f79b50103067 Jakob Hauser 2022-08-13 407 if (yas5xx->version == YAS537_VERSION_1) {
65f79b50103067 Jakob Hauser 2022-08-13 408 for (i = 0; i < 3; i++)
65f79b50103067 Jakob Hauser 2022-08-13 409 s[i] = xy1y2[i] - BIT(13);
65f79b50103067 Jakob Hauser 2022-08-13 410 h[0] = (c->k * (128 * s[0] + c->a2 * s[1] + c->a3 * s[2])) / BIT(13);
65f79b50103067 Jakob Hauser 2022-08-13 411 h[1] = (c->k * (c->a4 * s[0] + c->a5 * s[1] + c->a6 * s[2])) / BIT(13);
65f79b50103067 Jakob Hauser 2022-08-13 412 h[2] = (c->k * (c->a7 * s[0] + c->a8 * s[1] + c->a9 * s[2])) / BIT(13);
65f79b50103067 Jakob Hauser 2022-08-13 413 for (i = 0; i < 3; i++) {
65f79b50103067 Jakob Hauser 2022-08-13 @414 clamp_val(h[i], -BIT(13), BIT(13) - 1);
65f79b50103067 Jakob Hauser 2022-08-13 415 xy1y2[i] = h[i] + BIT(13);
65f79b50103067 Jakob Hauser 2022-08-13 416 }
65f79b50103067 Jakob Hauser 2022-08-13 417 }
65f79b50103067 Jakob Hauser 2022-08-13 418
65f79b50103067 Jakob Hauser 2022-08-13 419 *x = xy1y2[0];
65f79b50103067 Jakob Hauser 2022-08-13 420 *y1 = xy1y2[1];
65f79b50103067 Jakob Hauser 2022-08-13 421 *y2 = xy1y2[2];
65f79b50103067 Jakob Hauser 2022-08-13 422
65f79b50103067 Jakob Hauser 2022-08-13 423 return 0;
65f79b50103067 Jakob Hauser 2022-08-13 424
65f79b50103067 Jakob Hauser 2022-08-13 425 out_unlock:
65f79b50103067 Jakob Hauser 2022-08-13 426 mutex_unlock(&yas5xx->lock);
65f79b50103067 Jakob Hauser 2022-08-13 427 return ret;
65f79b50103067 Jakob Hauser 2022-08-13 428 }
65f79b50103067 Jakob Hauser 2022-08-13 429
From: kernel test robot > Sent: 28 November 2024 15:05 > > Hi David, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on next-20241121] > > url: https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-h-Add-whitespace-around- > operators-and-after-commas/20241121-152617 > base: next-20241121 > patch link: https://lore.kernel.org/r/8f69f4deac014f558bab186444bac2e8%40AcuMS.aculab.com > patch subject: [PATCH next 6/7] minmax.h: Simplify the variants of clamp() > config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241128/202411282222.oF0B4110- > lkp@intel.com/config) > compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project > 592c0fe55f6d9a811028b5f3507be91458ab2713) > reproduce (this is a W=1 build): (https://download.01.org/0day- > ci/archive/20241128/202411282222.oF0B4110-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202411282222.oF0B4110-lkp@intel.com/ ... > vim +414 drivers/iio/magnetometer/yamaha-yas530.c ... > 65f79b50103067 Jakob Hauser 2022-08-13 407 if (yas5xx->version == YAS537_VERSION_1) { > 65f79b50103067 Jakob Hauser 2022-08-13 408 for (i = 0; i < 3; i++) > 65f79b50103067 Jakob Hauser 2022-08-13 409 s[i] = xy1y2[i] - BIT(13); > 65f79b50103067 Jakob Hauser 2022-08-13 410 h[0] = (c->k * (128 * s[0] + c->a2 * s[1] + c-> >a3 * s[2])) / BIT(13); > 65f79b50103067 Jakob Hauser 2022-08-13 411 h[1] = (c->k * (c->a4 * s[0] + c->a5 * s[1] + c-> >a6 * s[2])) / BIT(13); > 65f79b50103067 Jakob Hauser 2022-08-13 412 h[2] = (c->k * (c->a7 * s[0] + c->a8 * s[1] + c-> >a9 * s[2])) / BIT(13); > 65f79b50103067 Jakob Hauser 2022-08-13 413 for (i = 0; i < 3; i++) { > 65f79b50103067 Jakob Hauser 2022-08-13 @414 clamp_val(h[i], -BIT(13), BIT(13) - 1); > 65f79b50103067 Jakob Hauser 2022-08-13 415 xy1y2[i] = h[i] + BIT(13); > 65f79b50103067 Jakob Hauser 2022-08-13 416 } > 65f79b50103067 Jakob Hauser 2022-08-13 417 } Duplicate, very buggy code... It very much needs BIT(13) to be signed. And it shouldn't ignore the result of clamp() David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h index 75fb7a6ad4c6..2bbdd5b5e07e 100644 --- a/include/linux/minmax.h +++ b/include/linux/minmax.h @@ -183,29 +183,29 @@ #define __clamp(val, lo, hi) \ ((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val))) -#define __clamp_once(val, lo, hi, uval, ulo, uhi) ({ \ - __auto_type uval = (val); \ - __auto_type ulo = (lo); \ - __auto_type uhi = (hi); \ +#define __clamp_once(type, val, lo, hi, uval, ulo, uhi) ({ \ + type uval = (val); \ + type ulo = (lo); \ + type uhi = (hi); \ BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \ "clamp() low limit " #lo " greater than high limit " #hi); \ BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi), \ "clamp("#val", "#lo", "#hi") signedness error"); \ __clamp(uval, ulo, uhi); }) -#define __careful_clamp(val, lo, hi) \ - __clamp_once(val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_)) +#define __careful_clamp(type, val, lo, hi) \ + __clamp_once(type, val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_)) /** - * clamp - return a value clamped to a given range with strict typechecking + * clamp - return a value clamped to a given range with typechecking * @val: current value * @lo: lowest allowable value * @hi: highest allowable value * - * This macro does strict typechecking of @lo/@hi to make sure they are of the - * same type as @val. See the unnecessary pointer comparisons. + * This macro checks @val/@lo/@hi to make sure they have compatible + * signedness. */ -#define clamp(val, lo, hi) __careful_clamp(val, lo, hi) +#define clamp(val, lo, hi) __careful_clamp(__auto_type, val, lo, hi) /** * clamp_t - return a value clamped to a given range using a given type @@ -217,7 +217,7 @@ * This macro does no typechecking and uses temporary variables of type * @type to make all the comparisons. */ -#define clamp_t(type, val, lo, hi) __careful_clamp((type)(val), (type)(lo), (type)(hi)) +#define clamp_t(type, val, lo, hi) __careful_clamp(type, val, lo, hi) /** * clamp_val - return a value clamped to a given range using val's type @@ -230,7 +230,7 @@ * type and @lo and @hi are literals that will otherwise be assigned a signed * integer type. */ -#define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi) +#define clamp_val(val, lo, hi) __careful_clamp(typeof(val), val, lo, hi) /* * Do not check the array parameter using __must_be_array().
Always pass a 'type' through to __clamp_once(), pass '__auto_type' from clamp() itself. The expansion of __types_ok3() is reasonable so it isn't worth the added complexity of avoiding it when a fixed type is used for all three values. Signed-off-by: David Laight <david.laight@aculab.com> --- include/linux/minmax.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)