On 03/02/2025 at 22:59, Geert Uytterhoeven wrote: > Hi Vincent, > > On Mon, 3 Feb 2025 at 14:37, Vincent Mailhol <mailhol.vincent@xxxxxxxxxx> wrote: >> On 03/02/2025 at 16:44, Johannes Berg wrote: >>> On Sun, 2025-02-02 at 12:53 -0500, Yury Norov wrote: >>>>> Instead of creating another variant for >>>>> non-constant bitfields, wouldn't it be better to make the existing macro >>>>> accept both? >>>> >>>> Yes, it would definitely be better IMO. >>> >>> On the flip side, there have been discussions in the past (though I >>> think not all, if any, on the list(s)) about the argument order. Since >>> the value is typically not a constant, requiring the mask to be a >>> constant has ensured that the argument order isn't as easily mixed up as >>> otherwise. >> >> If this is a concern, then it can be checked with: >> >> BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask) && >> __builtin_constant_p(_val), >> _pfx "mask is not constant"); >> >> It means that we forbid FIELD_PREP(non_const_mask, const_val) but allow >> any other combination. > > Even that case looks valid to me. Actually there is already such a user > in drivers/iio/temperature/mlx90614.c: > > ret |= field_prep(chip_info->fir_config_mask, MLX90614_CONST_FIR); > > So if you want enhanced safety, having both the safer/const upper-case > variants and the less-safe/non-const lower-case variants makes sense. So, we are scared of people calling FIELD_PREP() with the arguments in the wrong order: FIELD_PREP(val, mask) thus adding the check that mask must be a compile time constant. But if we introduce a second function, don't we introduce the risk of having people use the lower case variant instead of the upper case variant? field_prep(incorrect_const_mask, val) I am not sure to follow the logic of why having two functions is the safer choice. Whatever the solution you propose, there will be a way to misuse it. Let me ask, what is the most likely to happen: 1. wrong parameter order 2. wrong function name ? If you have the conviction that people more often do mistake 1. then I am fine with your solution. Otherwise, if 1. and 2. have an equally likelihood, then I would argue to go with the simplicity of the single function. Yours sincerely, Vincent Mailhol
