diff mbox series

[05/21] trailer: rename 'free_all' to 'free_all_trailer_items'

Message ID 20201025212652.3003036-6-anders@0x63.nu
State New
Headers show
Series trailer fixes | expand

Commit Message

Anders Waldenborg Oct. 25, 2020, 9:26 p.m. UTC
); SAEximRunCond expanded to false

No functional change intended.

Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
 trailer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Christian Couder Oct. 26, 2020, 12:42 p.m. UTC | #1
On Sun, Oct 25, 2020 at 10:27 PM Anders Waldenborg <anders@0x63.nu> wrote:
>
> ); SAEximRunCond expanded to false

As already mentioned, please find a way to remove the above line in
all your patches.

> No functional change intended.

This doesn't explain much why renaming 'free_all' to
'free_all_trailer_items' is a good idea. Is the function specific to
trailer items or is it generic enough to be useful on other 'struct
list_head *head'?

> Signed-off-by: Anders Waldenborg <anders@0x63.nu>
Jeff King Nov. 10, 2020, 7:52 p.m. UTC | #2
On Mon, Oct 26, 2020 at 01:42:25PM +0100, Christian Couder wrote:

> > No functional change intended.
> 
> This doesn't explain much why renaming 'free_all' to
> 'free_all_trailer_items' is a good idea. Is the function specific to
> trailer items or is it generic enough to be useful on other 'struct
> list_head *head'?

It can't be generic, because list_head needs to use the expected
containing type to find the containing pointer. So free_all() is quite a
bad name, even within trailer.c, because the compiler won't even tell
you if you pass a different list to it.

I do agree this should be spelled out in the commit message, though. :)

-Peff
diff mbox series

Patch

diff --git a/trailer.c b/trailer.c
index 8c0687a529..227df1c0ef 100644
--- a/trailer.c
+++ b/trailer.c
@@ -990,7 +990,7 @@  static size_t process_input_file(FILE *outfile,
 	return info.trailer_end - str;
 }
 
-static void free_all(struct list_head *head)
+static void free_all_trailer_items(struct list_head *head)
 {
 	struct list_head *pos, *p;
 	list_for_each_safe(pos, p, head) {
@@ -1057,7 +1057,7 @@  void process_trailers(const char *file,
 
 	print_all(outfile, &head, opts);
 
-	free_all(&head);
+	free_all_trailer_items(&head);
 
 	/* Print the lines after the trailers as is */
 	if (!opts->only_trailers)