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

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

From
erik elfström <erik.elfstrom@gmail.com>
Date
Apr 28, 2015, 20:07 UTC
Message-ID
<CAMpP7Nb3aiMoTjtPJNJPv38G54ZawS8B+NDX0x2iNe6FA-L+Lw@mail.gmail.com>
In-Reply-To
<20150428061741.GL24580@peff.net>
On Tue, Apr 28, 2015 at 8:17 AM, Jeff King <peff@peff.net> wrote:
Show 13 quoted lines
>
> There was a discussion not too long ago on strategies for returning
> errors, and one of the suggestions was to return an "error strbuf"
> rather than a code[1]. That's less flexible, as the caller can't react
> differently based on the type of error. But for cases like this, where
> the only fate for the code is to get converted back into a message,
> it can reduce the boilerplate.
>
> What you have here is OK to me, and I don't want to hold up your patch
> series in a flamewar about error-reporting techniques. But I think it's
> an interesting case study.
>
> -Peff

Thanks. I haven't had time to look through that thread yet, I'll try to get to that later.

My initial reaction is a bit skeptical though. For this case we currently don't want any error reporting, the NULL return is sufficient and even allocating and sending in the int* is pure noise. Allocating and releasing a strbuf seems like a lot more overhead for this type of caller? The one other potential candidate caller for read_gitfile_gently that I have seen (clone.c:get_repo_path) don't want any error code or message either as far as i can tell.

Also if it turns out that we actually need to treat the "file too large" error differently in clean (as discussed in thread on the file size check) then we can no longer communicate that back using the strbuf interface.

Overall it seems like a less attractive solution to me but I can be persuaded if there is more support expressed for the strbuf direction.

/Erik
Previous: Jeff KingNext: Jeff King
Message 12 of 38 in “Improving performance of git clean”
  1. 0/5 Improving performance of git cleanErik Elfström, Apr 25, 2015
  2. 1/5 setup: add gentle version of read_gitfileErik Elfström, Apr 25, 2015
  3. Junio C HamanoApr 25, 2015
  4. Junio C HamanoApr 25, 2015
  5. 2/5 setup: sanity check file size in read_gitfile_gentlyErik Elfström, Apr 25, 2015
  6. Junio C HamanoApr 25, 2015
  7. Erik ElfströmApr 25, 2015
  8. Junio C HamanoApr 26, 2015
  9. 0/5 Improving performance of git cleanErik Elfström, Apr 26, 2015
  10. 1/5 setup: add gentle version of read_gitfileErik Elfström, Apr 26, 2015
  11. Jeff KingApr 28, 2015
  12. erik elfströmApr 28, 2015
  13. Jeff KingApr 28, 2015
  14. Jonathan NiederApr 28, 2015
  15. Jeff KingApr 28, 2015
  16. Jonathan NiederApr 28, 2015
  17. Jeff KingApr 28, 2015
  18. Jonathan NiederApr 28, 2015
  19. Junio C HamanoApr 28, 2015
  20. Stefan BellerApr 29, 2015
  21. Junio C HamanoApr 30, 2015
  22. 2/5 setup: sanity check file size in read_gitfile_gentlyErik Elfström, Apr 26, 2015
  23. Jeff KingApr 28, 2015
  24. Windows path limites, was Re: [PATCH v5 2/5] setup: sanity check file size in read_gitfile_gentlyJohannes Schindelin, Apr 28, 2015
  25. Doug KellyApr 28, 2015
  26. Johannes SchindelinApr 28, 2015
  27. erik elfströmApr 28, 2015
  28. Junio C HamanoApr 29, 2015
  29. 3/5 t7300: add tests to document behavior of clean and nested gitErik Elfström, Apr 26, 2015
  30. 4/5 p7300: add performance tests for cleanErik Elfström, Apr 26, 2015
  31. Jeff KingApr 28, 2015
  32. erik elfströmApr 28, 2015
  33. 5/5 clean: improve performance when removing lots of directoriesErik Elfström, Apr 26, 2015
  34. Jeff KingApr 28, 2015
  35. erik elfströmApr 28, 2015
  36. 3/5 t7300: add tests to document behavior of clean and nested gitErik Elfström, Apr 25, 2015
  37. 4/5 p7300: add performance tests for cleanErik Elfström, Apr 25, 2015
  38. 5/5 clean: improve performance when removing lots of directoriesErik Elfström, Apr 25, 2015

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.