On Thu, Jul 5, 2018 at 12:49 PM, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > On Thu, Jul 05, 2018 at 07:46:05AM -0700, Dan Williams wrote: >> On Thu, Jul 5, 2018 at 1:29 AM, Johannes Thumshirn <jthumshirn@xxxxxxx> wrote: >> > On Wed, Jul 04, 2018 at 11:50:13PM -0700, Dan Williams wrote: >> >> +static ssize_t memmap_state_store(struct device *dev, >> >> + struct device_attribute *attr, const char *buf, size_t len) >> >> +{ >> >> + int i; >> >> + struct nd_pfn *nd_pfn = to_nd_pfn_safe(dev); >> >> + struct memmap_async_state *async = &nd_pfn->async; >> >> + >> >> + if (strcmp(buf, "sync") == 0) >> >> + /* pass */; >> >> + else if (strcmp(buf, "sync\n") == 0) >> >> + /* pass */; >> >> + else >> >> + return -EINVAL; >> > >> > Hmm what about: >> > >> > if (strncmp(buf, "sync", 4)) >> > return -EINVAL; >> > >> > This collapses 6 lines into 4. >> >> ...but that also allows 'echo "syncAndThenSomeGarbage" > >> /sys/.../memmap_state' to succeed. > > if (strncmp(buf, "sync", 4)) > return -EINVAL; > if (buf[4] != '\0' && buf[4] != '\n') > return -EINVAL; > Not sure that's a win either, I'd rather just: + if (strcmp(buf, "sync") == 0 || strcmp(buf, "sync\n") == 0) + /* pass */; + else + return -EINVAL; If we're trying to save those 2 lines.