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, 18:52 UTC
Message-ID
<20150916185256.GA2654@sigill.intra.peff.net>
In-Reply-To
<xmqqh9muql2c.fsf@gitster.mtv.corp.google.com>
On Wed, Sep 16, 2015 at 11:24:27AM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Sep 15, 2015 at 11:19:21PM -0400, Eric Sunshine wrote:
> >
> >> >                 strcpy(hexbuf[stage], sha1_to_hex(ce->sha1));
> >> > -               sprintf(ownbuf[stage], "%o", ce->ce_mode);
> >> > +               xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
> >> 
> >> Interesting. I wonder if there are any (old/broken) compilers which
> >> would barf on this. If we care, perhaps sizeof(ownbuf[0]) instead?
> >
> > Good point. I've changed it to sizeof(ownbuf[0]).
> 
> Panda brain is lost here.  What's the difference, other than that we
> will now appear to be measuring the size of the thing at index 0
> while using that size to stuff data into a different location?  All
> elements of the array are of the same size so there wouldn't be any
> difference either way, no?

Correct. The original is sane and gcc does the right thing. The question is whether some compiler would complain that "stage" is not a constant in the sizeof() expression. I don't know if any compiler would do so, but it is easy enough to be conservative.

-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 52 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.