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

[PATCH 5/6] trailer: make trailer_info struct private

From
LGLinus Arver via GitGitGadget <gitgitgadget@gmail.com>
Date
Mar 16, 2024, 06:27 UTC
Message-ID
<cf59dee506441a11b2b295d046a7bf255ca7c1cf.1710570428.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1696.git.1710570428.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>

In 13211ae23f (trailer: separate public from internal portion of trailer_iterator, 2023-09-09) we moved trailer_info behind an anonymous struct to discourage use by trailer.h API users. However it still left open the possibility of external use of trailer_info itself. Now that there are no external users of trailer_info, we can make this struct private.

Make this struct private by putting its definition inside trailer.c. This has two benefits:

  (1) it makes the surface area of the public facing
      interface (trailer.h) smaller, and
  (2) external API users are unable to peer inside this struct (because
      it is only ever exposed as an opaque pointer).
There are a couple disadvantages:
  (A) every time the member of the struct is accessed an extra pointer
      dereference must be done, and
  (B) for users of trailer_info outside trailer.c, this struct can no
      longer be allocated on the stack and may only be allocated on the
      heap (because its definition is hidden away in trailer.c) and
      appropriately deallocated by the user.

(The disadvantages have already been observed in the two preparatory commits that precede this one.) This commit believes that the benefits outweigh the disadvantages for designing APIs, as explained below.

Making trailer_info private exposes existing deficiencies in the API. This is because users of this struct had full access to its internals, so there wasn't much need to actually design it to be "complete" in the sense that API users only needed to use what was provided by the API. For example, the location of the trailer block (start/end offsets relative to the start of the input text) was accessible by looking at these struct members directly. Now that the struct is private, we have to expose new API functions to allow clients to access this information (see builtin/interpret-trailers.c).

The idea in this commit to hide implementation details behind an "opaque pointer" is also known as the "pimpl" (pointer to implementation) idiom in C++ and is a common pattern in that language (where, for example, abstract classes only have pointers to concrete classes).

However, the original inspiration to use this idiom does not come from C++, but instead the book "C Interfaces and Implementations: Techniques for Creating Reusable Software" [1]. This book recommends opaque pointers as a good design principle for designing C libraries, using the term "interface" as the functions defined in *.h (header) files and "implementation" as the corresponding *.c file which define the interfaces.

The book says this about opaque pointers:
    ... clients can manipulate such pointers freely, but they can’t
    dereference them; that is, they can’t look at the innards of the
    structure pointed to by them. Only the implementation has that
    privilege. Opaque pointers hide representation details and help
    catch errors.

In our case, "struct trailer_info" is now hidden from clients, and the ways in which this opaque pointer can be used is limited to the richness of <trailer.h>. In other words, <trailer.h> exclusively controls exactly how "trailer_info" pointers are to be used.

[1] Hanson, David R. "C Interfaces and Implementations: Techniques for
    Creating Reusable Software". Addison Wesley, 1997. p. 22
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 21 +++++++++++++++++++++
 trailer.h | 23 ++---------------------
 2 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/trailer.c b/trailer.c
index 9179dd802c6..6167b707ae0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -11,6 +11,27 @@
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
 
+struct trailer_info {
+	/*
+	 * True if there is a blank line before the location pointed to by
+	 * trailer_block_start.
+	 */
+	int blank_line_before_trailer;
+
+	/*
+	 * Offsets to the trailer block start and end positions in the input
+	 * string. If no trailer block is found, these are both set to the
+	 * "true" end of the input (find_end_of_log_message()).
+	 */
+	size_t trailer_block_start, trailer_block_end;
+
+	/*
+	 * Array of trailers found.
+	 */
+	char **trailers;
+	size_t trailer_nr;
+};
+
 struct conf_info {
 	char *name;
 	char *key;
diff --git a/trailer.h b/trailer.h
index b32213a9e23..a63e97a2663 100644
--- a/trailer.h
+++ b/trailer.h
@@ -4,6 +4,8 @@
 #include "list.h"
 #include "strbuf.h"
 
+struct trailer_info;
+
 enum trailer_where {
 	WHERE_DEFAULT,
 	WHERE_END,
@@ -29,27 +31,6 @@ int trailer_set_where(enum trailer_where *item, const char *value);
 int trailer_set_if_exists(enum trailer_if_exists *item, const char *value);
 int trailer_set_if_missing(enum trailer_if_missing *item, const char *value);
 
-struct trailer_info {
-	/*
-	 * True if there is a blank line before the location pointed to by
-	 * trailer_block_start.
-	 */
-	int blank_line_before_trailer;
-
-	/*
-	 * Offsets to the trailer block start and end positions in the input
-	 * string. If no trailer block is found, these are both set to the
-	 * "true" end of the input (find_end_of_log_message()).
-	 */
-	size_t trailer_block_start, trailer_block_end;
-
-	/*
-	 * Array of trailers found.
-	 */
-	char **trailers;
-	size_t trailer_nr;
-};
-
 /*
  * A list that represents newly-added trailers, such as those provided
  * with the --trailer command line option of git-interpret-trailers.
-- 
gitgitgadget
Previous: Linus Arver via GitGitGadgetNext: Linus Arver via GitGitGadget
Message 6 of 66 in “Make trailer_info struct private (plus sequencer cleanup)”
  1. 0/6 Make trailer_info struct private (plus sequencer cleanup)Linus Arver via GitGitGadget, Mar 16, 2024
  2. 1/6 trailer: teach iterator about non-trailer linesLinus Arver via GitGitGadget, Mar 16, 2024
  3. 2/6 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, Mar 16, 2024
  4. 3/6 interpret-trailers: access trailer_info with new helpersLinus Arver via GitGitGadget, Mar 16, 2024
  5. 4/6 trailer: make parse_trailers() return trailer_info pointerLinus Arver via GitGitGadget, Mar 16, 2024
  6. 5/6 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, Mar 16, 2024
  7. 6/6 trailer: retire trailer_info_get() from APILinus Arver via GitGitGadget, Mar 16, 2024
  8. Junio C HamanoMar 16, 2024
  9. Junio C HamanoMar 26, 2024
  10. Linus ArverApr 19, 2024
  11. 0/8 Make trailer_info struct private (plus sequencer cleanup)Linus Arver via GitGitGadget, Apr 19, 2024
  12. 1/8 Makefile: sort UNIT_TEST_PROGRAMSLinus Arver via GitGitGadget, Apr 19, 2024
  13. 2/8 trailer: add unit tests for trailer iteratorLinus Arver via GitGitGadget, Apr 19, 2024
  14. Linus ArverApr 19, 2024
  15. Linus ArverApr 19, 2024
  16. Junio C HamanoApr 19, 2024
  17. Linus ArverApr 20, 2024
  18. 3/8 trailer: teach iterator about non-trailer linesLinus Arver via GitGitGadget, Apr 19, 2024
  19. 4/8 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, Apr 19, 2024
  20. Junio C HamanoApr 23, 2024
  21. 5/8 interpret-trailers: access trailer_info with new helpersLinus Arver via GitGitGadget, Apr 19, 2024
  22. 7/8 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, Apr 19, 2024
  23. Junio C HamanoApr 23, 2024
  24. Linus ArverApr 25, 2024
  25. 6/8 trailer: make parse_trailers() return trailer_info pointerLinus Arver via GitGitGadget, Apr 19, 2024
  26. Junio C HamanoApr 23, 2024
  27. 8/8 trailer: retire trailer_info_get() from APILinus Arver via GitGitGadget, Apr 19, 2024
  28. Junio C HamanoApr 23, 2024
  29. Junio C HamanoApr 24, 2024
  30. 00/10 Make trailer_info struct private (plus sequencer cleanup)Linus Arver via GitGitGadget, Apr 26, 2024
  31. 01/10 Makefile: sort UNIT_TEST_PROGRAMSLinus Arver via GitGitGadget, Apr 26, 2024
  32. 02/10 trailer: add unit tests for trailer iteratorLinus Arver via GitGitGadget, Apr 26, 2024
  33. Christian CouderApr 26, 2024
  34. Junio C HamanoApr 26, 2024
  35. Linus ArverApr 26, 2024
  36. 03/10 trailer: teach iterator about non-trailer linesLinus Arver via GitGitGadget, Apr 26, 2024
  37. Christian CouderApr 27, 2024
  38. Linus ArverApr 30, 2024
  39. Linus ArverApr 30, 2024
  40. 04/10 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, Apr 26, 2024
  41. 05/10 interpret-trailers: access trailer_info with new helpersLinus Arver via GitGitGadget, Apr 26, 2024
  42. 06/10 trailer: make parse_trailers() return trailer_info pointerLinus Arver via GitGitGadget, Apr 26, 2024
  43. 07/10 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, Apr 26, 2024
  44. 08/10 trailer: retire trailer_info_get() from APILinus Arver via GitGitGadget, Apr 26, 2024
  45. 10/10 trailer unit tests: inspect iterator contentsLinus Arver via GitGitGadget, Apr 26, 2024
  46. 09/10 trailer: document parse_trailers() usageLinus Arver via GitGitGadget, Apr 26, 2024
  47. Christian CouderApr 27, 2024
  48. 00/10 Make trailer_info struct private (plus sequencer cleanup)Linus Arver via GitGitGadget, May 2, 2024
  49. 01/10 Makefile: sort UNIT_TEST_PROGRAMSLinus Arver via GitGitGadget, May 2, 2024
  50. 02/10 trailer: add unit tests for trailer iteratorLinus Arver via GitGitGadget, May 2, 2024
  51. Junio C HamanoMay 2, 2024
  52. 03/10 trailer: teach iterator about non-trailer linesLinus Arver via GitGitGadget, May 2, 2024
  53. Phillip WoodMay 4, 2024
  54. Linus ArverMay 5, 2024
  55. Phillip WoodMay 5, 2024
  56. Linus ArverMay 9, 2024
  57. Phillip WoodMay 13, 2024
  58. Phillip WoodMay 13, 2024
  59. 04/10 sequencer: use the trailer iteratorLinus Arver via GitGitGadget, May 2, 2024
  60. 05/10 interpret-trailers: access trailer_info with new helpersLinus Arver via GitGitGadget, May 2, 2024
  61. 06/10 trailer: make parse_trailers() return trailer_info pointerLinus Arver via GitGitGadget, May 2, 2024
  62. 07/10 trailer: make trailer_info struct privateLinus Arver via GitGitGadget, May 2, 2024
  63. 08/10 trailer: retire trailer_info_get() from APILinus Arver via GitGitGadget, May 2, 2024
  64. 09/10 trailer: document parse_trailers() usageLinus Arver via GitGitGadget, May 2, 2024
  65. 10/10 trailer unit tests: inspect iterator contentsLinus Arver via GitGitGadget, May 2, 2024
  66. Junio C HamanoMay 2, 2024

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.