On 2019-02-26 07:08, Pankaj Bansal wrote: >>> + if (IS_ERR_OR_NULL(regmap)) { >>> + ret = PTR_ERR_OR_ZERO(regmap) ? PTR_ERR(regmap) : - >> ENODEV; >> >> The above is not correct, this should be better (untested): >> >> ret = PTR_ERR(regmap) ?: -ENODEV; > > Omitting the second operand in ternary operator is not standard. > https://stackoverflow.com/questions/34559705/ternary-operator-without-the-middle-expression
Irrelevant, a great many non-standard features are used in the kernel. > Although, it *has been* used in kernel in many places > https://livegrep.com/search/linux?q=file%3A%5C.c%24%20%5C%3F%5C%3A&fold_case=auto®ex=true&context=true Yes, a local grep suggests that there are plenty of instances in the kernel. In fact, there are multiple instances even in the kernel/ directory so it's not some fringe thing. My conclusion is that it makes for readable code and is perfectly safe to use in the kernel, which apparently does not care about compilers that do not understand the construct. $ git grep '?:' *.[ch] | wc 773 4830 54975 I you still don't buy that and don't want to sign-off on it, then write ret = IS_ERR(regmap) ? PTR_ERR(regmap) : -ENODEV; But that's just wordy and less readable, methinks. However, the best thing to do, according to your maintainer, is still the original suggestion to fixup the NULL as early as possible with regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-ENODEV); And leave the error handling block alone. Can you please just do that and be done with it? Cheers, Peter