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

> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to