On Wed, Jan 18, 2023 at 03:04:23PM +0100, Marijn Suijten wrote: > On 2023-01-18 15:22:42, Andy Shevchenko wrote: ... > > > > + name = devm_kasprintf(dev, GFP_KERNEL, "%pfwP", fwnode); > > > > > > Is this better/cleaner than copying the string from fwnode_get_name? > > > > Coying to where? And what would be the lifetime of that string? > > > > With devm_kasprintf(): > > - we don't care how long the string is > > - we don't care about corner cases of lifetime as it's the same as > > device itself (i.o.w. the same as the IIO device container) > > Curious if there isn't a devm_strdup(name) or similar? Main point is > that %pfwP seems like magic when fwnode_get_name is not (but returns a > const string that we cannot modify). The devm_kstrdup(fwnode_get_name()) is an open coded variant of the above. I don't think we need to open code and produce NIH even a single API. And no, there is no magic behind that. At least from the fwnode point of view. You may very well say that > 1500 instances of "%pOF" is a magic... > If there is not, let's stick with > devm_kasprintf(). There is, but I'm against it. See above why. > > > > + name[strchrnul(name, '@') - name] = '\0'; > > > > > > This is the same as *strchrnul(name, '@') = '\0'; if I'm not mistaken. > > > > Yes, But it's harder to read and understand. I believe the compiler has > > enough power to optimize this to the same assembly code. > > I find the latter clearer as it doesn't require the reader to figure out > that name - name cancels itself out. Alternatively we can write > strchrnul(name, '@')[0]. I don't like to have Pythonisms in the C code, really. P.S. I guess this little patch already emptied my bandwidth, so I leave any further discussion to you and IIO maintainers. Thank you for the review! -- With Best Regards, Andy Shevchenko
