On Fri, 2018-12-21 at 18:25 -0500, Steven Rostedt wrote: > On Fri, 21 Dec 2018 15:19:33 -0800 > Joe Perches <[email protected]> wrote: > > > I believe this should be bool. > > > > I don't find a use for non-zero assigned len value in the kernel > > for strncmp and I believe the function should simply be: > > > > static inline bool str_has_prefix(const char *str, const char prefix[]) > > { > > return !strncmp(str, prefix, strlen(prefix)); > > } > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c [] > @@ -172,8 +172,8 @@ cache_type_store(struct device *dev, struct > device_attribute *attr, > * it's not worth the risk */ > return -EINVAL; > > - if (strncmp(buf, temp, sizeof(temp) - 1) == 0) { > - buf += sizeof(temp) - 1; > + if ((len = str_has_prefix(buf, temp))) { > + buf += len;
That's not really a use of the non-zero strncmp return value. You are attempting an optimization not already done. I also wonder if it's actually an optimization as the return value may not be precomputed. Also the assignment in the test isn't preferred style. > And there's more places like this. Any where the non-zero return value is actually used? > > It's hard to believe __always_inline vs inline matters > > for any single line function. > > I've been burnt by gcc deciding to not inline single functions before. Complex single functions sure, but single line inlines? I haven't seen that externed anywhere. Today no inline function is marked __always_inline in string.h I don't doubt there should be some standardization of inline vs __always_inline in the kernel, but this right now seems different just for difference sake. cheers, Joe
