Re: [PATCH] git-compat-util.h:rewrite skip_prefix() as loop

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 2, 2014, 05:00 UTC
Message-ID: <CAPig+cRDHb9usJW2zC20zXXM_9KxuEQ+=dkFmghp9JiVPcL1_w@mail.gmail.com>
In-Reply-To: <1393680724-19983-1-git-send-email-siddharth98391@gmail.com>

Thanks for the submission. Minor comments below to give you a taste of what it's like to contribute to this project...

On Sat, Mar 1, 2014 at 8:32 AM, Siddharth Goel <siddharth98391@gmail.com> wrote:

> Rewrote skip_prefix() function so that prefix is scanned once.

Good description. In this project, use imperative tone, so say "Rewrite skip_prefix()..." as you did in the subject. In fact, this description is short enough and conveys sufficient information that it could just be placed in the subject as the entire commit message.

    Subject: skip_prefix: rewrite so that prefix is scanned once

Show 25 quoted lines

> Signed-off-by: Siddharth Goel <siddharth98391@gmail.com>
> ---
>  git-compat-util.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 614a5e9..550dce3 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -357,8 +357,11 @@ extern int suffixcmp(const char *str, const char *suffix);
>
>  static inline const char *skip_prefix(const char *str, const char *prefix)
>  {
> -       size_t len = strlen(prefix);
> -       return strncmp(str, prefix, len) ? NULL : str + len;
> +       while (*prefix != '\0' && *str == *prefix) {
> +               str++;
> +               prefix++;
> +       }
> +       return (*prefix == '\0' ? str : NULL);
>  }
>
>  #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
> --
> 1.9.0.138.g2de3478.dirty