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

Re: [PATCH v2 21/21] t1308: add a test case on open a config directory

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
May 3, 2017, 15:33 UTC
Message-ID
<alpine.DEB.2.20.1705031730040.3480@virtualbox>
In-Reply-To
<20170503101706.9223-22-pclouds@gmail.com>
Hi Duy,
On Wed, 3 May 2017, Nguyễn Thái Ngọc Duy wrote:
Show 6 quoted lines
> We don't support config-as-a-directory (maybe someday we will?). Make
> sure we consistently fail in this case, which should happen on platforms
> where fopen(<directory>) returns non-NULL if FREAD_READS_DIRECTORIES is
> defined.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

How about saying that we make sure that this is the case by adding a test case? Otherwise the reader might (and this reader certainly did) expect a code change in a .c file.

I reviewed this patch series, and am happy with the overall diff.

My main problem: it should be organized in a more intuitively-graspable way, i.e. put all the fopen() -> fopen_or_warn() changes into a single patch, separate out changes (drive-by test fixing for Windows and, ahem, whitespace changes) into their own commits. Then those commits should be ordered to relate an easy-flowing story.

Other than that: well done, Dscho

Previous: Nguyễn Thái Ngọc DuyNext: Nguyễn Thái Ngọc Duy
Message 64 of 70 in “Handle fopen() errors”
  1. 00/15 Handle fopen() errorsNguyễn Thái Ngọc Duy, Apr 20, 2017
  2. 02/15 bisect: report on fopen() errorNguyễn Thái Ngọc Duy, Apr 20, 2017
  3. 03/15 blame: report error on open if graft_file is a directoryNguyễn Thái Ngọc Duy, Apr 20, 2017
  4. 01/15 config.mak.uname: set FREAD_READS_DIRECTORIES for Linux and FreeBSDNguyễn Thái Ngọc Duy, Apr 20, 2017
  5. 05/15 log: report errno on failure to fopen() a fileNguyễn Thái Ngọc Duy, Apr 20, 2017
  6. Jeff KingApr 21, 2017
  7. 06/15 commit.c: report error on failure to fopen() the graft fileNguyễn Thái Ngọc Duy, Apr 20, 2017
  8. 09/15 rerere.c: report correct errnoNguyễn Thái Ngọc Duy, Apr 20, 2017
  9. 04/15 clone: use xfopen() instead of fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  10. 10/15 sequencer.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  11. 08/15 rerere.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  12. 11/15 server-info: report error on failure to fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  13. 07/15 remote.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  14. Johannes SixtApr 26, 2017
  15. Junio C HamanoApr 27, 2017
  16. Johannes SixtApr 27, 2017
  17. Duy NguyenApr 27, 2017
  18. Johannes SixtApr 27, 2017
  19. 12/15 wt-status.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  20. 15/15 t1308: add a test case on open a config directoryNguyễn Thái Ngọc Duy, Apr 20, 2017
  21. 14/15 config.c: handle error on failing to fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  22. 13/15 xdiff-interface.c: report errno on failure to stat() or fopen()Nguyễn Thái Ngọc Duy, Apr 20, 2017
  23. Junio C HamanoApr 21, 2017
  24. Junio C HamanoApr 21, 2017
  25. Jeff KingApr 21, 2017
  26. Duy NguyenApr 21, 2017
  27. Junio C HamanoApr 21, 2017
  28. Duy NguyenApr 21, 2017
  29. Jeff KingApr 21, 2017
  30. Junio C HamanoApr 24, 2017
  31. 00/21 Nguyễn Thái Ngọc Duy, May 3, 2017
  32. 01/21 Use xfopen() in more placesNguyễn Thái Ngọc Duy, May 3, 2017
  33. 03/21 config.mak.uname: set FREAD_READS_DIRECTORIES for Linux and FreeBSDNguyễn Thái Ngọc Duy, May 3, 2017
  34. 02/21 clone: use xfopen() instead of fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  35. Johannes SchindelinMay 3, 2017
  36. 04/21 wrapper.c: add warn_on_fopen_errors()Nguyễn Thái Ngọc Duy, May 3, 2017
  37. Johannes SchindelinMay 3, 2017
  38. Junio C HamanoMay 4, 2017
  39. 05/21 wrapper.c: add fopen_or_warn()Nguyễn Thái Ngọc Duy, May 3, 2017
  40. 06/21 attr.c: use fopen_or_warn()Nguyễn Thái Ngọc Duy, May 3, 2017
  41. Johannes SchindelinMay 3, 2017
  42. 07/21 ident.c: use fopen_or_warn()Nguyễn Thái Ngọc Duy, May 3, 2017
  43. 08/21 bisect: report on fopen() errorNguyễn Thái Ngọc Duy, May 3, 2017
  44. Johannes SchindelinMay 3, 2017
  45. 09/21 blame: report error on open if graft_file is a directoryNguyễn Thái Ngọc Duy, May 3, 2017
  46. Johannes SchindelinMay 3, 2017
  47. 10/21 log: report errno on failure to fopen() a fileNguyễn Thái Ngọc Duy, May 3, 2017
  48. Johannes SchindelinMay 3, 2017
  49. 11/21 log: fix memory leak in open_next_file()Nguyễn Thái Ngọc Duy, May 3, 2017
  50. 12/21 commit.c: report error on failure to fopen() the graft fileNguyễn Thái Ngọc Duy, May 3, 2017
  51. 13/21 remote.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  52. Johannes SchindelinMay 3, 2017
  53. Duy NguyenMay 9, 2017
  54. Johannes SchindelinMay 9, 2017
  55. 14/21 rerere.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  56. 15/21 rerere.c: report correct errnoNguyễn Thái Ngọc Duy, May 3, 2017
  57. Johannes SchindelinMay 3, 2017
  58. 17/21 server-info: report error on failure to fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  59. 16/21 sequencer.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  60. 18/21 wt-status.c: report error on failure to fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  61. 20/21 config.c: handle error on failing to fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  62. Johannes SchindelinMay 3, 2017
  63. 21/21 t1308: add a test case on open a config directoryNguyễn Thái Ngọc Duy, May 3, 2017
  64. Johannes SchindelinMay 3, 2017
  65. 19/21 xdiff-interface.c: report errno on failure to stat() or fopen()Nguyễn Thái Ngọc Duy, May 3, 2017
  66. Junio C HamanoMay 4, 2017
  67. Junio C HamanoMay 7, 2017
  68. Duy NguyenMay 9, 2017
  69. Junio C HamanoMay 9, 2017
  70. config.mak.uname: set FREAD_READS_DIRECTORIES for Darwin, tooJunio C Hamano, May 10, 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.