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

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

From
Jeff King <peff@peff.net>
Date
Sep 16, 2015, 09:45 UTC
Message-ID
<20150916094517.GA13966@sigill.intra.peff.net>
In-Reply-To
<CAGZ79kb5qWnOnJGY8JR3Z9UqT2xp390A6-LvNudBk=e3N2W41Q@mail.gmail.com>
On Tue, Sep 15, 2015 at 01:38:42PM -0700, Stefan Beller wrote:
Show 5 quoted lines
> Some off topic thoughts:
> 
> Having an "assert" behavior is not a good user experience though
> and should be fixed. To fix it we need stack traces. And the git
> version. And telling the user to send it to the mailing list.

Yes, any assert (or die("BUG")) should generally result in a code change to make it never happen again.

I have been tempted to replace our die("BUG") calls with a BUG() macro, so that we can do more fancy things (even if it is just adding __FILE__:__LINE__ and a message to contact the list).

> I wonder if we want to include a trace where possible (i.e.
> when compiled with gcc or other precompiler conditions)
> into these assertive behaviors.
That would be nice. I took a look at this back in April:
  http://thread.gmane.org/gmane.comp.version-control.git/267643/focus=267755

but I think I convinced myself that we are better off relying on people running gdb. If we do anything, it should be to make doing so easier.

> I'd guess we have an assertive behavior if die("BUG:...") is called,
> so maybe we can just check inside of die if we want to print
> a stack trace additionally ?

I guess we could look for starts_with(fmt, "BUG: ") in die(). I think it would be a bit less gross to convert those calls to a BUG() macro (we can't get accurate __LINE__ information without it, though in practice the die messages are usually unique enough to be helpful).

I think a core file is even more useful than a backtrace. Having BUG() call abort() would be even more useful, I think.

-Peff
Previous: Stefan BellerNext: Junio C Hamano
Message 46 of 154 in “war on sprintf, strcpy, etc”
  1. 0/67 war on sprintf, strcpy, etcJeff King, Sep 15, 2015
  2. 01/67 show-branch: avoid segfault with --reflog of unborn branchJeff King, Sep 15, 2015
  3. 02/67 mailsplit: fix FILE* leak in split_maildirJeff King, Sep 15, 2015
  4. 03/67 archive-tar: fix minor indentation violationJeff King, Sep 15, 2015
  5. 04/67 fsck: don't fsck alternates for connectivity-only checkJeff King, Sep 15, 2015
  6. Johannes SchindelinSep 15, 2015
  7. Junio C HamanoSep 16, 2015
  8. Jeff KingSep 16, 2015
  9. Junio C HamanoSep 16, 2015
  10. Eric SunshineSep 16, 2015
  11. Jeff KingSep 16, 2015
  12. 05/67 add xsnprintf helper functionJeff King, Sep 15, 2015
  13. 06/67 add git_path_buf helper functionJeff King, Sep 15, 2015
  14. 07/67 strbuf: make strbuf_complete_line more genericJeff King, Sep 15, 2015
  15. Eric SunshineSep 16, 2015
  16. Junio C HamanoSep 16, 2015
  17. Jeff KingSep 16, 2015
  18. Eric SunshineSep 16, 2015
  19. 08/67 add reentrant variants of sha1_to_hex and find_unique_abbrevJeff King, Sep 15, 2015
  20. Ramsay JonesSep 15, 2015
  21. Jeff KingSep 15, 2015
  22. Junio C HamanoSep 16, 2015
  23. Johannes SchindelinSep 16, 2015
  24. Jeff KingSep 16, 2015
  25. Junio C HamanoSep 16, 2015
  26. Jeff KingSep 16, 2015
  27. 09/67 fsck: use strbuf to generate alternate directoriesJeff King, Sep 15, 2015
  28. 10/67 mailsplit: make PATH_MAX buffers dynamicJeff King, Sep 15, 2015
  29. Eric SunshineSep 16, 2015
  30. Jeff KingSep 16, 2015
  31. Jeff KingSep 16, 2015
  32. Junio C HamanoSep 16, 2015
  33. Jeff KingSep 16, 2015
  34. 11/67 trace: use strbuf for quote_crnl outputJeff King, Sep 15, 2015
  35. Eric SunshineSep 16, 2015
  36. Jeff KingSep 16, 2015
  37. Eric SunshineSep 16, 2015
  38. 12/67 progress: store throughput display in a strbufJeff King, Sep 15, 2015
  39. 13/67 test-dump-cache-tree: avoid overflow of cache-tree nameJeff King, Sep 15, 2015
  40. 14/67 compat/inet_ntop: fix off-by-one in inet_ntop4Jeff King, Sep 15, 2015
  41. 15/67 convert trivial sprintf / strcpy calls to xsnprintfJeff King, Sep 15, 2015
  42. Ramsay JonesSep 15, 2015
  43. Jeff KingSep 15, 2015
  44. Ramsay JonesSep 15, 2015
  45. Stefan BellerSep 15, 2015
  46. Jeff KingSep 16, 2015
  47. Junio C HamanoSep 16, 2015
  48. Junio C HamanoSep 16, 2015
  49. Eric SunshineSep 16, 2015
  50. Jeff KingSep 16, 2015
  51. Junio C HamanoSep 16, 2015
  52. Jeff KingSep 16, 2015
  53. Junio C HamanoSep 16, 2015
  54. Stefan BellerSep 16, 2015
  55. Jeff KingSep 16, 2015
  56. Jeff KingSep 16, 2015
  57. 16/67 archive-tar: use xsnprintf for trivial formattingJeff King, Sep 15, 2015
  58. 17/67 use xsnprintf for generating git object headersJeff King, Sep 15, 2015
  59. Junio C HamanoSep 16, 2015
  60. 18/67 find_short_object_filename: convert sprintf to xsnprintfJeff King, Sep 15, 2015
  61. 19/67 stop_progress_msg: convert sprintf to xsnprintfJeff King, Sep 15, 2015
  62. 20/67 compat/hstrerror: convert sprintf to snprintfJeff King, Sep 15, 2015
  63. 21/67 grep: use xsnprintf to format failure messageJeff King, Sep 15, 2015
  64. 22/67 entry.c: convert strcpy to xsnprintfJeff King, Sep 15, 2015
  65. Ramsay JonesSep 15, 2015
  66. Stefan BellerSep 15, 2015
  67. 23/67 add_packed_git: convert strcpy into xsnprintfJeff King, Sep 15, 2015
  68. Junio C HamanoSep 16, 2015
  69. Jeff KingSep 16, 2015
  70. 24/67 http-push: replace strcat with xsnprintfJeff King, Sep 15, 2015
  71. 25/67 receive-pack: convert strncpy to xsnprintfJeff King, Sep 15, 2015
  72. 26/67 replace trivial malloc + sprintf /strcpy calls to xstrfmtJeff King, Sep 15, 2015
  73. Eric SunshineSep 16, 2015
  74. Jeff KingSep 16, 2015
  75. 27/67 config: use xstrfmt in normalize_valueJeff King, Sep 15, 2015
  76. 28/67 fetch: replace static buffer with xstrfmtJeff King, Sep 15, 2015
  77. 29/67 use strip_suffix and xstrfmt to replace suffixJeff King, Sep 15, 2015
  78. Eric SunshineSep 16, 2015
  79. Jeff KingSep 16, 2015
  80. Eric SunshineSep 16, 2015
  81. 30/67 ref-filter: drop sprintf and strcpy callsJeff King, Sep 15, 2015
  82. Junio C HamanoSep 16, 2015
  83. 31/67 help: drop prepend function in favor of xstrfmtJeff King, Sep 15, 2015
  84. 32/67 mailmap: replace strcpy with xstrdupJeff King, Sep 15, 2015
  85. 33/67 read_branches_file: replace strcpy with xstrdupJeff King, Sep 15, 2015
  86. Junio C HamanoSep 16, 2015
  87. Jeff KingSep 16, 2015
  88. Jeff KingSep 17, 2015
  89. Jeff KingSep 17, 2015
  90. Jeff KingSep 17, 2015
  91. Junio C HamanoSep 17, 2015
  92. Jeff KingSep 17, 2015
  93. Junio C HamanoSep 17, 2015
  94. 34/67 resolve_ref: use strbufs for internal buffersJeff King, Sep 15, 2015
  95. 35/67 upload-archive: convert sprintf to strbufJeff King, Sep 15, 2015
  96. 36/67 remote-ext: simplify git pkt-line generationJeff King, Sep 15, 2015
  97. Junio C HamanoSep 16, 2015
  98. Jeff KingSep 16, 2015
  99. 37/67 http-push: use strbuf instead of fwrite_bufferJeff King, Sep 15, 2015
  100. 38/67 http-walker: store url in a strbufJeff King, Sep 15, 2015
  101. 39/67 sha1_get_pack_name: use a strbufJeff King, Sep 15, 2015
  102. 40/67 init: use strbufs to store pathsJeff King, Sep 15, 2015
  103. 41/67 apply: convert root string to strbufJeff King, Sep 15, 2015
  104. 42/67 transport: use strbufs for status table "quickref" stringsJeff King, Sep 15, 2015
  105. 43/67 merge-recursive: convert malloc / strcpy to strbufJeff King, Sep 15, 2015
  106. 44/67 enter_repo: convert fixed-size buffers to strbufsJeff King, Sep 15, 2015
  107. 45/67 remove_leading_path: use a strbuf for internal storageJeff King, Sep 15, 2015
  108. 46/67 write_loose_object: convert to strbufJeff King, Sep 15, 2015
  109. Junio C HamanoSep 16, 2015
  110. Jeff KingSep 16, 2015
  111. 47/67 diagnose_invalid_index_path: use strbuf to avoid strcpy/strcatJeff King, Sep 15, 2015
  112. 48/67 fetch-pack: use argv_array for index-pack / unpack-objectsJeff King, Sep 15, 2015
  113. 49/67 http-push: use an argv_array for setup_revisionsJeff King, Sep 15, 2015
  114. 50/67 stat_tracking_info: convert to argv_arrayJeff King, Sep 15, 2015
  115. 51/67 daemon: use cld->env_array when re-spawningJeff King, Sep 15, 2015
  116. 52/67 use sha1_to_hex_to() instead of strcpyJeff King, Sep 15, 2015
  117. Junio C HamanoSep 16, 2015
  118. Jeff KingSep 16, 2015
  119. Junio C HamanoSep 16, 2015
  120. 53/67 drop strcpy in favor of raw sha1_to_hexJeff King, Sep 15, 2015
  121. Eric SunshineSep 18, 2015
  122. Jeff KingSep 18, 2015
  123. 54/67 color: add overflow checks for parsing colorsJeff King, Sep 15, 2015
  124. Eric SunshineSep 18, 2015
  125. Jeff KingSep 18, 2015
  126. Junio C HamanoSep 21, 2015
  127. 55/67 use alloc_ref rather than hand-allocating "struct ref"Jeff King, Sep 15, 2015
  128. 56/67 avoid sprintf and strcpy with flex arraysJeff King, Sep 15, 2015
  129. Eric SunshineSep 20, 2015
  130. Jeff KingSep 21, 2015
  131. Eric SunshineSep 21, 2015
  132. Jeff KingSep 21, 2015
  133. 57/67 receive-pack: simplify keep_arg computationJeff King, Sep 15, 2015
  134. Eric SunshineSep 18, 2015
  135. Jeff KingSep 18, 2015
  136. 58/67 help: clean up kfmclient mungingJeff King, Sep 15, 2015
  137. 59/67 prefer memcpy to strcpyJeff King, Sep 15, 2015
  138. 60/67 color: add color_set helper for copying raw colorsJeff King, Sep 15, 2015
  139. 61/67 notes: document length of fanout path with a constantJeff King, Sep 15, 2015
  140. 62/67 convert strncpy to memcpyJeff King, Sep 15, 2015
  141. 63/67 fsck: drop inode-sorting codeJeff King, Sep 15, 2015
  142. 64/67 Makefile: drop D_INO_IN_DIRENT build knobJeff King, Sep 15, 2015
  143. 65/67 fsck: use for_each_loose_file_in_objdirJeff King, Sep 15, 2015
  144. 66/67 use strbuf_complete to conditionally append slashJeff King, Sep 15, 2015
  145. Junio C HamanoSep 16, 2015
  146. Jeff KingSep 16, 2015
  147. Junio C HamanoSep 16, 2015
  148. Jeff KingSep 16, 2015
  149. Junio C HamanoSep 17, 2015
  150. Eric SunshineSep 21, 2015
  151. Jeff KingSep 21, 2015
  152. 67/67 name-rev: use strip_suffix to avoid magic numbersJeff King, Sep 15, 2015
  153. Junio C HamanoSep 16, 2015
  154. Jeff KingSep 16, 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.