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

Re: [PATCH 2/3] merge-recursive: remove return value from get_files_dirs

From
Jeff King <peff@peff.net>
Date
Aug 29, 2017, 08:17 UTC
Message-ID
<20170829081752.nq5r776rjyf2amzh@sigill.intra.peff.net>
In-Reply-To
<20170828202829.3056-3-kewillf@microsoft.com>
On Mon, Aug 28, 2017 at 02:28:28PM -0600, Kevin Willford wrote:
Show 6 quoted lines
> The return value of the get_files_dirs call is never being used.
> Looking at the history of the file and it was originally only
> being used for debug output statements.  Also when
> read_tree_recursive return value is non zero it is changed to
> zero.  This leads me to believe that it doesn't matter if
> read_tree_recursive gets an error.
Or that the function is buggy. :)

I'm tempted to say that we should probably die() when read_tree_recursive fails. This should only happen if we fail to parse the tree, or if our callback (save_files_dirs here) returns failure, and the latter looks like it never happens.

> Since the debug output has been removed and the caller isn't
> checking the return value there is no reason to keep calulating
> and returning a value.
Agreed, and I'm happy to see dead code go.
Minor nit: s/calulating/calculating/ in your commit message.
-Peff
Previous: Jeff KingNext: Kevin Willford
Message 8 of 15 in “merge-recursive: replace string_list with hashmap”
  1. 0/3 merge-recursive: replace string_list with hashmapKevin Willford, Aug 28, 2017
  2. 1/3 merge-recursive: fix memory leakKevin Willford, Aug 28, 2017
  3. Ben PeartAug 28, 2017
  4. Jeff KingAug 29, 2017
  5. 2/3 merge-recursive: remove return value from get_files_dirsKevin Willford, Aug 28, 2017
  6. Ben PeartAug 28, 2017
  7. Jeff KingAug 29, 2017
  8. Jeff KingAug 29, 2017
  9. Kevin WillfordAug 29, 2017
  10. Jeff KingAug 29, 2017
  11. Stefan BellerAug 31, 2017
  12. 3/3 merge-recursive: change current file dir string_lists to hashmapKevin Willford, Aug 28, 2017
  13. Ben PeartAug 28, 2017
  14. Jeff KingAug 29, 2017
  15. Junio C HamanoSep 6, 2017

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.