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

Re: [PATCH] reftable: pass pq_entry by address

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 13, 2022, 15:55 UTC
Message-ID
<xmqqbkrjb75g.fsf@gitster.g>
In-Reply-To
<183353220fe.d7826593472673.3445243727369286065@elijahpepe.com>
Elijah Conners <business@elijahpepe.com> writes:
> In merged_iter_pqueue_add, the pq_entry parameter is passed by value,
> although it exceeds 64 bytes.

Do we have any hard guidance like "do not pass an data item whose size is larger than 64 bytes" in our coding guidelines? If not, make sure that the reference to 64 bytes does not look like one.

While this patch is not bad as a change, we are going to make a copy of the value with structure assignment at the leaf level, I am not sure how big a deal this is in practice.

In any case, wouldn't it make sense to make the "we pass reference not because we want to let the callee modify the value, but because the callee deep in the callchain wants to copy the contents out of it" parameter a pointer to a constant? I.e.

    void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry *e)
Other than that, looking good.
Previous: Han-Wen NienhuysNext: Elijah Conners
Message 3 of 9 in “reftable: pass pq_entry by address”
  1. reftable: pass pq_entry by addressElijah Conners, Sep 13, 2022
  2. Han-Wen NienhuysSep 13, 2022
  3. Junio C HamanoSep 13, 2022
  4. Elijah ConnersSep 13, 2022
  5. Han-Wen NienhuysSep 13, 2022
  6. Elijah ConnersSep 13, 2022
  7. Han-Wen NienhuysSep 15, 2022
  8. Junio C HamanoSep 15, 2022
  9. reftable: use const with the pq_entry paramElijah Conners, Sep 14, 2022

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.