Re: [PATCH] implemented strbuf_write_or_die()

> On Mon, Mar 3, 2014 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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.
>
> As a potential GSoC student and newcomer to the project, Faiz would
> not have known that this would be considered unwanted churn when he
> chose the task from the GSoC microproject page [1]. Perhaps it would
> be a good idea to retire this item from the list?

> On the other hand, it did expose Faiz to the iterative code review
> process on this project and gave him a taste of what would be expected
> of him as a GSoC student, so the microproject achieved that important
> goal, and thus wasn't an utter failure.
>
> [1]: https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md