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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 28, 2014, 21:59 UTC
Message-ID: <CAPig+cTGL+s5nT9rgcHhsM+oPTjoJPaorFKHmYtGJ=ZCk4Rm=A@mail.gmail.com>
In-Reply-To: <CAFbjVcnx41CTOV-SCGC9a91b9weNTgYoRP1+uVDukp6Z+0F_yg@mail.gmail.com>

On Fri, Feb 28, 2014 at 1:27 PM, Faiz Kothari <faiz.off93@gmail.com> 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.

Show 19 quoted lines

> 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.