Re: [PATCH] rewrite bulk-checkin.c:finish_bulk_checkin() using strbuf

On Fri, Feb 28, 2014 at 1:27 PM, Faiz Kothari [off-list ref] wrote:

Thanks for the suggestions and remarks.

[Administrivia: On this list, top-posting is frowned upon; inline
responses are preferred.]

I rewrote bulk-checkin.c:finish_bulk_checkin() using strbuf. But saw
that Sun He has already implemented the same way I have done.
Should I submit my implementation as a patch?

Yes. The purpose of these micro-projects is to expose you to the Git
project's development process so that you know what will be expected
of you as a GSoC student, and to give the GSoC mentors an opportunity
to evaluate your abilities and observe how you interact with the
reviewers.

Secondly,
I tried implementing this WITHOUT changing the prototype of the
function pack-write.c:finish_tmp_packfile().

For this I detached the buffer from strbuf in finish_bulk_checkin()
using strbuf_detach() and passed it to finish_tmp_packfile().

Inside finish_tmp_packfile, I attached the same buffer to a local
struct strbuf using strbuf_attach().
Now the problem is, two of the arguments to strbuf_attach() are
'alloc' and 'len' which are private members of the struct strbuf.
But since I am just passing the detached buffer, the information of
alloc and len is lost which is required at the time of attaching.
I cannot think of any better way of using strbuf and NOT modify the
prototype of finish_tmp_packfile()

As a workaround, I can determine alloc = (strlen(buf) + 1) and len =
strlen(buf) but AFAIK this is not always true and may break.
Any suggestions?

That's getting rather convoluted. You may want to ask yourself if it
is really necessary for finish_tmp_packfile() to modify the buffer
passed in by the caller or if finish_pack_file() should instead take
responsibility for itself by allocating its own buffer (strbuf) in
which to do path manipulation.