On Fri, Feb 28, 2014 at 1:27 PM, Faiz Kothari <faiz.off93@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html