Re: [PATCH v5 1/2] mm: add kstrimdup function

Quoting Andrew Morton (2014-01-30 13:22:51)

On Thu, 30 Jan 2014 13:11:57 -0800 Sebastian Capella sebastian.capella@linaro.org wrote:

...

+char *kstrimdup(const char *s, gfp_t gfp) +{

• char *buf;
  

• char *begin = skip_spaces(s);
  

• size_t len = strlen(begin);
  

• while (len > 1 && isspace(begin[len - 1]))
  

•         len--;
  

That's off-by-one isn't it? kstrimdup(" ") should return "", not " ".

...

• buf = kmalloc_track_caller(len + 1, gfp);
  

• if (!buf)
  

•         return NULL;
  

• memcpy(buf, begin, len);
  

• buf[len] = '\0';
  

• return buf;
  

+}

Hi Andrew,

I think this is a little tricky.

For an empty string, the function relies on skip_spaces to point begin at the \0'.

Alternately, if we don't have an empty string, we know we have at least 1 non-space, non-null character at begin[0], and there's no need to check it, so the loop stops at [1]. If there's a space at 1, we just put the '\0' there.

We could check at [0], but I think its already been checked by skip_spaces.

I'll add a comment above the while for that

Thanks,

Sebastian