Re: [PATCH 04/16] refactor skip_prefix to return a boolean

On Thu, Jun 19, 2014 at 09:59:39PM -0400, Eric Sunshine wrote:

quoted

diff --git a/git-compat-util.h b/git-compat-util.h
index b6f03b3..556c839 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -349,13 +349,31 @@ extern void set_die_is_recursing_routine(int (*routine)(void));
 extern int starts_with(const char *str, const char *prefix);
 extern int ends_with(const char *str, const char *suffix);

-static inline const char *skip_prefix(const char *str, const char *prefix)
+/*
+ * If "str" begins with "prefix", return 1. If out is non-NULL,
+ * it it set to str + strlen(prefix) (i.e., the prefix is skipped).

The documentation claims that 'out' can be NULL, however, the code
does not respect this. NULL 'out' seems rather pointless (unless you
want an alias for starts_with()), so presumably the documentation is
incorrect.

Thanks for catching. My original version (before I sent to the list) did
allow for a conditional NULL out, but I realized there was exactly one
caller who wanted this, and that they would be better off using
starts_with. I added patch 3 ("avoid using skip_prefix as a boolean")
and stripped the conditional from skip_prefix, but forgot to update the
comment.

I think we'd just want to squash the patch below (I also took the
opportunity to fix a typo and clarify the text a bit):

diff --git a/git-compat-util.h b/git-compat-util.h
index 556c839..1187e1a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -350,8 +350,9 @@ extern int starts_with(const char *str, const char *prefix);
 extern int ends_with(const char *str, const char *suffix);
 
 /*
- * If "str" begins with "prefix", return 1. If out is non-NULL,
- * it it set to str + strlen(prefix) (i.e., the prefix is skipped).
+ * If the string "str" begins with the string found in "prefix", return 1.
+ * The "out" parameter is set to "str + strlen(prefix)" (i.e., to the point in
+ * the string right after the prefix).
  *
  * Otherwise, returns 0 and out is left untouched.
  *

-Peff