On Sun, Feb 22, 2015 at 11:33:16PM +0100, René Scharfe wrote: > Am 22.02.2015 um 21:00 schrieb Junio C Hamano: > >René Scharfe <l.s.r@xxxxxx> writes: > > > >>Use strlcpy() instead of calling strncpy() and then setting the last > >>byte of the target buffer to NUL explicitly. This shortens and > >>simplifies the code a bit. > > > >Thanks. It makes me wonder if the longer term direction should be > >not to use a bound buffer for oc->path, though. > > That's a good idea in general, but a bit more involved since we'd need to > introduce a cleanup function that releases the memory allocated by the new > version of get_sha1_with_context() first and call it from the appropriate > places. > > Would that be a good micro-project for GSoC or is it too simple? Yeah, avoiding resource ownership questions was one of the reasons I went with the static buffer in the first place. But I would love to see it go away. Not only does it potentially truncate paths, but I recall there was some complication with the size of "struct object_context" (I couldn't find the details in a cursory search, but basically it was not reasonable to have a big array of them). Could we perhaps make this more like sha1_object_info_extended, where the caller "asks" for fields by filling in pointers, and the object_context itself can be discarded without leaking resources? Like: struct strbuf path = STRBUF_INIT; struct object_context oc = OBJECT_CONTEXT_INIT; oc.path = &path; get_sha1_with_context(sha1, &oc); ... use path directly ... strbuf_release(&path); Then callers who do not care about the path do not have to even know the feature exists (and it opens us up to adding new string-like context fields in the future if we need to). -Peff -- 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