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

Re: [PATCH] implemented strbuf_write_or_die()

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Mar 4, 2014, 09:18 UTC
Message-ID
<53159A81.4050905@alum.mit.edu>
In-Reply-To
<xmqqvbvvqglc.fsf@gitster.dls.corp.google.com>
On 03/03/2014 07:31 PM, Junio C Hamano wrote:
Show 29 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
>> On Sat, Mar 1, 2014 at 7:51 AM, He Sun <sunheehnus@gmail.com> wrote:
>>> 2014-03-01 19:21 GMT+08:00 Faiz Kothari <faiz.off93@gmail.com>:
>>>> diff --git a/remote-curl.c b/remote-curl.c
>>>> index 10cb011..dee8716 100644
>>>> --- a/remote-curl.c
>>>> +++ b/remote-curl.c
>>>> @@ -634,7 +634,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
>>>>         if (start_command(&client))
>>>>                 exit(1);
>>>>         if (preamble)
>>>> -               write_or_die(client.in, preamble->buf, preamble->len);
>>>> +               strbuf_write_or_die(client.in, preamble);
>>>>         if (heads)
>>>>                 write_or_die(client.in, heads->buf, heads->len);
>>>
>>> This should be changed. May be you can use Ctrl-F to search write_or_die().
>>> Or if you are using vim, use "/ and n" to find all.
>>
>> It's not obvious from the patch fragment, but 'heads' is not a strbuf,
>> so Faiz correctly left this invocation alone.
> 
> That is a very good sign why this change is merely a code-churn and
> not an improvement, isn't it?  We know (and any strbuf user should
> know) that ->buf and ->len are the ways to learn the pointer and the
> length the strbuf holds.  Why anybody thinks it is benefitial to
> introduce another function that is _only_ for writing out strbuf and
> cannot be used to write out a plain buffer is simply beyond me.

I'm the guilty one. I like the change (obviously, since I suggested it). Writing strbufs comes up frequently and will hopefully increase in usage and I think it is a positive thing to encourage the use of strbufs by making them increasingly first-class citizens.

But I can see your points too, and I humbly defer to the wisdom of the list. I will remove this suggestion from the list of microprojects.

Faiz, this is the way things go on the Git mailing list. It would be boring if everybody agreed all the time :-)

Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Previous: Eric SunshineNext: Faiz Kothari
Message 18 of 23 in “implemented strbuf_write_or_die()”
  1. implemented strbuf_write_or_die()Faiz Kothari, Mar 1, 2014
  2. He SunMar 1, 2014
  3. implemented strbuf_write_or_die()Faiz Kothari, Mar 1, 2014
  4. Michael HaggertyMar 1, 2014
  5. implemented strbuf_write_or_die()Faiz Kothari, Mar 2, 2014
  6. Eric SunshineMar 2, 2014
  7. Eric SunshineMar 2, 2014
  8. 1/2 Introduce strbuf_write_or_die()Faiz Kothari, Mar 2, 2014
  9. 2/2 use strbuf_write_or_die()Faiz Kothari, Mar 2, 2014
  10. Eric SunshineMar 2, 2014
  11. Eric SunshineMar 2, 2014
  12. Eric SunshineMar 2, 2014
  13. Junio C HamanoMar 3, 2014
  14. Eric SunshineMar 3, 2014
  15. David KastrupMar 3, 2014
  16. Junio C HamanoMar 3, 2014
  17. Eric SunshineMar 3, 2014
  18. Michael HaggertyMar 4, 2014
  19. Faiz KothariMar 4, 2014
  20. Junio C HamanoMar 4, 2014
  21. Johannes SixtMar 1, 2014
  22. Junio C HamanoMar 3, 2014
  23. Fwd: [PATCH] implemented strbuf_write_or_die()Faiz Kothari, Mar 3, 2014

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.