git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jun 20, 2014, 02:30 UTC
Message-ID
<CAPig+cST9MW2fYgCD3C-3HJQbMLHzBSX7Vxr23ejmVFTmkCj7A@mail.gmail.com>
In-Reply-To
<20140620020824.GA31908@sigill.intra.peff.net>
On Thu, Jun 19, 2014 at 10:08 PM, Jeff King <peff@peff.net> wrote:
Show 45 quoted lines
> On Thu, Jun 19, 2014 at 09:59:39PM -0400, Eric Sunshine wrote:
>
>> > 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.
For consistency with the preceding paragraph:
    Otherwise, return 0 and leave "out" untouched.
>   *
>
> -Peff
Previous: Jeff KingNext: Jeff King
Message 8 of 33 in “skip_prefix refactoring and cleanups”
  1. 0/16 skip_prefix refactoring and cleanupsJeff King, Jun 18, 2014
  2. 01/16 parse_diff_color_slot: drop ofs parameterJeff King, Jun 18, 2014
  3. 02/16 daemon: mark some strings as constJeff King, Jun 18, 2014
  4. 03/16 avoid using skip_prefix as a booleanJeff King, Jun 18, 2014
  5. 04/16 refactor skip_prefix to return a booleanJeff King, Jun 18, 2014
  6. Eric SunshineJun 20, 2014
  7. Jeff KingJun 20, 2014
  8. Eric SunshineJun 20, 2014
  9. Jeff KingJun 20, 2014
  10. Junio C HamanoJun 23, 2014
  11. Jeff KingJun 23, 2014
  12. builtin/clone.c: detect a clone starting at a tag correctlyJunio C Hamano, Jun 23, 2014
  13. 05/16 apply: use skip_prefix instead of raw additionJeff King, Jun 18, 2014
  14. 06/16 fast-import: fix read of uninitialized argv memoryJeff King, Jun 18, 2014
  15. 07/16 transport-helper: avoid reading past end-of-stringJeff King, Jun 18, 2014
  16. 08/16 use skip_prefix to avoid magic numbersJeff King, Jun 18, 2014
  17. Junio C HamanoJun 23, 2014
  18. Jeff KingJul 1, 2014
  19. 09/16 use skip_prefix to avoid repeating stringsJeff King, Jun 18, 2014
  20. 10/16 fast-import: use skip_prefix for parsing inputJeff King, Jun 18, 2014
  21. Eric SunshineJun 20, 2014
  22. Jeff KingJun 20, 2014
  23. Eric SunshineJun 20, 2014
  24. 11/16 daemon: use skip_prefix to avoid magic numbersJeff King, Jun 18, 2014
  25. 12/16 stat_opt: check extra strlen callJeff King, Jun 18, 2014
  26. 13/16 fast-import: refactor parsing of spacesJeff King, Jun 18, 2014
  27. 14/16 fetch-pack: refactor parsing in get_ackJeff King, Jun 18, 2014
  28. Eric SunshineJun 20, 2014
  29. 15/16 git: avoid magic number with skip_prefixJeff King, Jun 18, 2014
  30. 16/16 use skip_prefix to avoid repeated calculationsJeff King, Jun 18, 2014
  31. Tanay AbhraJun 19, 2014
  32. 17/16 http-push: refactor parsing of remote object namesJeff King, Jun 19, 2014
  33. Jeff KingJun 19, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.