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

Re: [PATCH v3 1/3] remote-curl: use strlen() instead of magic numbers

From
Jeff King <peff@peff.net>
Date
Jun 23, 2020, 18:54 UTC
Message-ID
<20200623185458.GD1444619@coredump.intra.peff.net>
In-Reply-To
<cb8683837c9f583274780057621255a65a1c4c9f.1592934880.git.liu.denton@gmail.com>
On Tue, Jun 23, 2020 at 01:55:32PM -0400, Denton Liu wrote:
> When we are dealing with a packet-line length header, we use the magic
> literal `4`, representing the length of "0000" and "0001", the packet
> line length headers. Use `strlen("000x")` so that we do not have to use
> the magic literal.

I'm not a huge fan of using strlen() for this, because it's _still_ magical (you cannot change "0000" in one place without changing it in another"). And while it helps with understanding that "4" matches the length of that string, IMHO it's harder to read because now I have to make sure that those much longer strings all match up.

This refactoring also implies to me that if you changed all of "0000" on one line you'd be fine, but that's emphatically not true. The magic number "4" is used to size the buffer earlier in the function, and would have to match (and of course since this is a network protocol, it's not like you could even change those in isolation).

So I dunno. I kind of think the raw "4" is the most readable. It's quite obvious to me in the context of a memcpy() what's going on. I don't mind memcpy_literal() or similar that hides the repetition, but I think it's hard to do here because of the arithmetic on the destination.

-Peff
Previous: Denton LiuNext: Junio C Hamano
Message 18 of 22 in “pkt-line: extract out PACKET_HEADER_SIZE”
  1. pkt-line: extract out PACKET_HEADER_SIZEDenton Liu, Jun 13, 2020
  2. Đoàn Trần Công DanhJun 13, 2020
  3. Denton LiuJun 13, 2020
  4. Junio C HamanoJun 13, 2020
  5. Junio C HamanoJun 14, 2020
  6. 0/3 pkt-line: war on magical `4` literalDenton Liu, Jun 14, 2020
  7. 2/3 pkt-line: use string versions of functionsDenton Liu, Jun 14, 2020
  8. Đoàn Trần Công DanhJun 14, 2020
  9. pkt-line: use string versions of functionsDenton Liu, Jun 14, 2020
  10. Junio C HamanoJun 14, 2020
  11. Denton LiuJun 14, 2020
  12. Đoàn Trần Công DanhJun 15, 2020
  13. 1/3 remote-curl: use strlen() instead of magic numbersDenton Liu, Jun 14, 2020
  14. 3/3 pkt-line: extract out PACKET_HEADER_SIZEDenton Liu, Jun 14, 2020
  15. Junio C HamanoJun 14, 2020
  16. 0/3 pkt-line: war on magical `4` literalDenton Liu, Jun 23, 2020
  17. 1/3 remote-curl: use strlen() instead of magic numbersDenton Liu, Jun 23, 2020
  18. Jeff KingJun 23, 2020
  19. Junio C HamanoJun 23, 2020
  20. 2/3 pkt-line: use string versions of functionsDenton Liu, Jun 23, 2020
  21. Jeff KingJun 23, 2020
  22. 3/3 pkt-line: extract out PACKET_HEADER_SIZEDenton Liu, Jun 23, 2020

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.