Re: [PATCH 00/12] Hard coded string length cleanup
- From
René Scharfe <l.s.r@web.de>
- Date
- Dec 20, 2013, 01:06 UTC
- Message-ID
- <52B397FF.4050808@web.de>
- In-Reply-To
- <CACsJy8Bb4+V1DEdEmRwj10Oozi8U430ZHDCj_UhnXZcaR-wQ=g@mail.gmail.com>
Am 20.12.2013 00:50, schrieb Duy Nguyen:
Show 18 quoted lines
> On Fri, Dec 20, 2013 at 6:32 AM, René Scharfe <l.s.r@web.de> wrote:
>> Seeing that skip_prefix_defval is mostly used in the form
>> skip_prefix_defval(foo, prefix, foo) I wonder if it makes sense to
>> first change skip_prefix to return the full string instead of NULL
>> if the prefix is not matched. Would the resulting function cover
>> most use cases? And would it still be easily usable?
>
> That was skip_prefix_gently() that I forgot to replace in a commit
> message, before I turned it into _defval variant. The reason for
> _defval is it could be use to chain expression together without adding
> temporary variables, e.g.
>
> - if (starts_with(line->buf, ">From") && isspace(line->buf[5])) {
> + if (isspace(*skip_prefix_defval(line->buf, ">From", "NOSPACE"))) {
>
> Without _defval, one would need to do if ((p = skip_prefix(..)) &&
> isspace(*p)). I'm not entirely sure this is a good thing though as it
> could make it a bit harder to read.That usage is quite rare compared to occurrences of skip_prefix_defval(foo, prefix, foo), no? Adding a temporary variable for them wouldn't be that bad if we can simplify the API to a single function -- if that one is usable, that is.
On the other hand, we could add a special function for that example and we'd already have three users in the tree (patch below). I think that's too narrow a use case for a library function, though. Doing the following instead in the three cases doesn't seem to be too bad:
rest = skip_prefix(line->buf, ">From");
if (rest != line->buf && isspace(*rest)) {--- builtin/apply.c | 2 +- builtin/mailinfo.c | 4 ++-- git-compat-util.h | 1 + strbuf.c | 9 +++++++++ 4 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c index b0d0986..b96befd 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -433,7 +433,7 @@ static unsigned long linelen(const char *buffer, unsigned long size) static int is_dev_null(const char *str) { - return !memcmp("/dev/null", str, 9) && isspace(str[9]); + return skip_prefix_and_space(str, "/dev/null") != str; } #define TERM_SPACE 1 diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c index 2c3cd8e..2575989 100644 --- a/builtin/mailinfo.c +++ b/builtin/mailinfo.c @@ -328,11 +328,11 @@ static int check_header(const struct strbuf *line, } /* for inbody stuff */ - if (starts_with(line->buf, ">From") && isspace(line->buf[5])) { + if (skip_prefix_and_space(line->buf, ">From") != line->buf) { ret = 1; /* Should this return 0? */ goto check_header_out; } - if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) { + if (skip_prefix_and_space(line->buf, "[PATCH]") != line->buf) { for (i = 0; header[i]; i++) { if (!memcmp("Subject", header[i], 7)) { handle_header(&hdr_data[i], line); diff --git a/git-compat-util.h b/git-compat-util.h index dcb92c4..a083918 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -355,6 +355,7 @@ extern int prefixcmp(const char *str, const char *prefix); extern int ends_with(const char *str, const char *suffix); extern int suffixcmp(const char *str, const char *suffix); extern const char *skip_prefix(const char *str, const char *prefix); +extern const char *skip_prefix_and_space(const char *str, const char *prefix); #if defined(NO_MMAP) || defined(USE_WIN32_MMAP) diff --git a/strbuf.c b/strbuf.c index 222df13..768331f 100644 --- a/strbuf.c +++ b/strbuf.c @@ -47,6 +47,15 @@ const char *skip_prefix(const char *str, const char *prefix) return str; } +const char *skip_prefix_and_space(const char *str, const char *prefix) +{ + const char *p = skip_prefix(str, prefix); + if (((p != str) || !*prefix) && isspace(*p)) + return p + 1; + else + return str; +} + /* * Used as the default ->buf value, so that people can always assume * buf is non NULL and ->buf is NUL terminated even for a freshly
-- 1.8.5.2