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

Re: [PATCH 06/21] convert manual allocations to argv_array

From
Jeff King <peff@peff.net>
Date
Feb 20, 2016, 08:34 UTC
Message-ID
<20160220083426.GA17171@sigill.intra.peff.net>
In-Reply-To
<CAPig+cTACku01im72Cd7LxYopwwFaXtKYXkxi53GMrTRyZpFGQ@mail.gmail.com>
On Sat, Feb 20, 2016 at 03:29:29AM -0500, Eric Sunshine wrote:
Show 29 quoted lines
> On Sat, Feb 20, 2016 at 3:10 AM, Jeff King <peff@peff.net> wrote:
> > On Sat, Feb 20, 2016 at 03:07:00AM -0500, Eric Sunshine wrote:
> >> > +               /* argv strings are now owned by pathspec */
> >> > +               paths.argc = 0;
> >> > +               argv_array_clear(&paths);
> >>
> >> This overly intimate knowledge of the internal implementation of
> >> argv_array_clear() is rather ugly.
> >
> > Yep, I agree. Suggestions?
> >
> > We can just leak the array of "char *". This function is called only
> > once per program invocation, and that's unlikely to change.
> >
> > I guess we can make an argv_array_detach_strings() function. Or maybe
> > even just argv_array_detach() would be less gross, and then this
> > function could manually free the array but not the strings themselves.
> 
> The latter is what I was thinking, and I agree that
> argv_array_detach() would be less gross than
> argv_array_detach_strings(), however, it also feels a bit wrong since
> this sort of ownership transfer is kind of out of scope for
> argv_array.
> 
> I wonder if a simple "dup'ing" string_list would be more suitable for
> this case. You'd have to append the NULL item manually with
> string_list_append_nodup(), and string_list_clear() would then be the
> correct way to dispose of the list without intimate knowledge of its
> implementation and no need for an API extension.

A string_list doesn't just store pointers; it's a struct with a util field. So you can't pass it to things expecting a "const char **".

I think argv_array_detach() is the least-bad thing here. It matches strbuf_detach() to say "you now own the storage" (as opposed to just peeking at argv.argv, which we should do only in a read-only way).

-Peff
Previous: Eric SunshineNext: Eric Sunshine
Message 49 of 93 in “hardening allocations against integer overflow”
  1. 0/18 hardening allocations against integer overflowJeff King, Feb 15, 2016
  2. 01/18 add helpers for detecting size_t overflowJeff King, Feb 15, 2016
  3. 02/18 tree-diff: catch integer overflow in combine_diff_path allocationJeff King, Feb 15, 2016
  4. 03/18 harden REALLOC_ARRAY and xcalloc against size_t overflowJeff King, Feb 15, 2016
  5. 04/18 add helpers for allocating flex-array structsJeff King, Feb 15, 2016
  6. Eric SunshineFeb 16, 2016
  7. Jeff KingFeb 16, 2016
  8. 05/18 convert trivial cases to ALLOC_ARRAYJeff King, Feb 15, 2016
  9. Eric SunshineFeb 16, 2016
  10. Jeff KingFeb 16, 2016
  11. Eric SunshineFeb 16, 2016
  12. Jeff KingFeb 16, 2016
  13. 06/18 use xmallocz to avoid size arithmeticJeff King, Feb 15, 2016
  14. 07/18 convert trivial cases to FLEX_ARRAY macrosJeff King, Feb 15, 2016
  15. Eric SunshineFeb 16, 2016
  16. Jeff KingFeb 16, 2016
  17. Jeff KingFeb 16, 2016
  18. Jeff KingFeb 16, 2016
  19. Eric SunshineFeb 16, 2016
  20. Jeff KingFeb 16, 2016
  21. Eric SunshineFeb 16, 2016
  22. 08/18 use st_add and st_mult for allocation size computationJeff King, Feb 15, 2016
  23. Eric SunshineFeb 16, 2016
  24. 09/18 write_untracked_extension: use FLEX_ALLOC helperJeff King, Feb 15, 2016
  25. 10/18 fast-import: simplify allocation in start_packfileJeff King, Feb 15, 2016
  26. 11/18 fetch-pack: simplify add_sought_entryJeff King, Feb 15, 2016
  27. 12/18 test-path-utils: fix normalize_path_copy output buffer sizeJeff King, Feb 15, 2016
  28. 13/18 sequencer: simplify memory allocation of get_messageJeff King, Feb 15, 2016
  29. Eric SunshineFeb 16, 2016
  30. 14/18 git-compat-util: drop mempcpy compat codeJeff King, Feb 15, 2016
  31. Eric SunshineFeb 16, 2016
  32. 15/18 transport_anonymize_url: use xstrfmtJeff King, Feb 15, 2016
  33. 16/18 diff_populate_gitlink: use a strbufJeff King, Feb 15, 2016
  34. 17/18 convert ewah/bitmap code to use xmallocJeff King, Feb 15, 2016
  35. 18/18 ewah: convert to REALLOC_ARRAY, etcJeff King, Feb 15, 2016
  36. Jeff KingFeb 15, 2016
  37. 0/21 hardening allocations against integer overflowJeff King, Feb 19, 2016
  38. 01/21 reflog_expire_cfg: NUL-terminate pattern fieldJeff King, Feb 19, 2016
  39. 02/21 add helpers for detecting size_t overflowJeff King, Feb 19, 2016
  40. 03/21 tree-diff: catch integer overflow in combine_diff_path allocationJeff King, Feb 19, 2016
  41. 04/21 harden REALLOC_ARRAY and xcalloc against size_t overflowJeff King, Feb 19, 2016
  42. René ScharfeFeb 20, 2016
  43. Jeff KingFeb 21, 2016
  44. 05/21 add helpers for allocating flex-array structsJeff King, Feb 19, 2016
  45. 06/21 convert manual allocations to argv_arrayJeff King, Feb 19, 2016
  46. Eric SunshineFeb 20, 2016
  47. Jeff KingFeb 20, 2016
  48. Eric SunshineFeb 20, 2016
  49. Jeff KingFeb 20, 2016
  50. Eric SunshineFeb 20, 2016
  51. Jeff KingFeb 20, 2016
  52. Eric SunshineFeb 20, 2016
  53. 07/21 convert trivial cases to ALLOC_ARRAYJeff King, Feb 19, 2016
  54. 08/21 use xmallocz to avoid size arithmeticJeff King, Feb 19, 2016
  55. 09/21 convert trivial cases to FLEX_ARRAY macrosJeff King, Feb 19, 2016
  56. 10/21 use st_add and st_mult for allocation size computationJeff King, Feb 19, 2016
  57. 11/21 prepare_{git,shell}_cmd: use argv_arrayJeff King, Feb 19, 2016
  58. 12/21 write_untracked_extension: use FLEX_ALLOC helperJeff King, Feb 19, 2016
  59. 13/21 fast-import: simplify allocation in start_packfileJeff King, Feb 19, 2016
  60. Junio C HamanoFeb 19, 2016
  61. Jeff KingFeb 19, 2016
  62. 14/21 fetch-pack: simplify add_sought_entryJeff King, Feb 19, 2016
  63. 15/21 test-path-utils: fix normalize_path_copy output buffer sizeJeff King, Feb 19, 2016
  64. 16/21 sequencer: simplify memory allocation of get_messageJeff King, Feb 19, 2016
  65. 17/21 git-compat-util: drop mempcpy compat codeJeff King, Feb 19, 2016
  66. 18/21 transport_anonymize_url: use xstrfmtJeff King, Feb 19, 2016
  67. 19/21 diff_populate_gitlink: use a strbufJeff King, Feb 19, 2016
  68. 20/21 convert ewah/bitmap code to use xmallocJeff King, Feb 19, 2016
  69. 21/21 ewah: convert to REALLOC_ARRAY, etcJeff King, Feb 19, 2016
  70. 0/22 hardening allocations against integer overflowJeff King, Feb 22, 2016
  71. 01/22 reflog_expire_cfg: NUL-terminate pattern fieldJeff King, Feb 22, 2016
  72. 02/22 add helpers for detecting size_t overflowJeff King, Feb 22, 2016
  73. 03/22 tree-diff: catch integer overflow in combine_diff_path allocationJeff King, Feb 22, 2016
  74. 04/22 harden REALLOC_ARRAY and xcalloc against size_t overflowJeff King, Feb 22, 2016
  75. 05/22 add helpers for allocating flex-array structsJeff King, Feb 22, 2016
  76. 06/22 argv-array: add detach functionJeff King, Feb 22, 2016
  77. 07/22 convert manual allocations to argv_arrayJeff King, Feb 22, 2016
  78. 08/22 convert trivial cases to ALLOC_ARRAYJeff King, Feb 22, 2016
  79. 09/22 use xmallocz to avoid size arithmeticJeff King, Feb 22, 2016
  80. 10/22 convert trivial cases to FLEX_ARRAY macrosJeff King, Feb 22, 2016
  81. 11/22 use st_add and st_mult for allocation size computationJeff King, Feb 22, 2016
  82. 12/22 prepare_{git,shell}_cmd: use argv_arrayJeff King, Feb 22, 2016
  83. 13/22 write_untracked_extension: use FLEX_ALLOC helperJeff King, Feb 22, 2016
  84. 14/22 fast-import: simplify allocation in start_packfileJeff King, Feb 22, 2016
  85. 15/22 fetch-pack: simplify add_sought_entryJeff King, Feb 22, 2016
  86. 16/22 test-path-utils: fix normalize_path_copy output buffer sizeJeff King, Feb 22, 2016
  87. 17/22 sequencer: simplify memory allocation of get_messageJeff King, Feb 22, 2016
  88. 18/22 git-compat-util: drop mempcpy compat codeJeff King, Feb 22, 2016
  89. 19/22 transport_anonymize_url: use xstrfmtJeff King, Feb 22, 2016
  90. 20/22 diff_populate_gitlink: use a strbufJeff King, Feb 22, 2016
  91. 21/22 convert ewah/bitmap code to use xmallocJeff King, Feb 22, 2016
  92. 22/22 ewah: convert to REALLOC_ARRAY, etcJeff King, Feb 22, 2016
  93. Junio C HamanoFeb 22, 2016

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.