Hello Jeff,
On 02-20-16, Jeff King wrote:
> On Sat, Feb 20, 2016 at 02:10:58PM +0600, Alexander Kuleshov wrote:
>
> I'm not convinced that moving the functions inline into git-compat-util
> is actually cleaner. We've expanded the interface that is visible to the
> whole code base, warts at all.
>
> One wart I see is that the caller cannot know whether the return value
> was newly allocated or not, and therefore cannot free it, creating a
> potential memory leak. Another is that the return value is not really
> necessary at all; we always munge argv[0].
>
> Does any other part of the code actually care about this
> extension-stripping?
Nope, only this one.
>
> Perhaps instead, could we do this:
> If we drop this default-to-empty value of STRIP_EXTENSION entirely, then
> we can do our #ifdef local to git.c, where it does not bother anybody
> else. Like:
>
> #ifdef STRIP_EXTENSION
> static void strip_extension(const char **argv)
> {
> /* Do the thing */
> }
> #else
> #define strip_extension(x)
> #endif
>
> (Note that I also simplified the return value).
>
> In the case that we do have STRIP_EXTENSION, I don't think we need to
> handle the empty-string case. It would be a regression for somebody
> passing -DSTRIP_EXTENSION="", but I don't think that's worth worrying
> about. That macro is defined totally internally.
>
> I suspect you could also use strip_suffix here. So something like:
>
> size_t len;
>
> if (strip_suffix(str, STRIP_EXTENSION, &len))
> argv[0] = xmemdupz(argv[0], len);
>
> would probably work, but that's totally untested.
Good suggestion. I will try to do it and test.
Thank you.
--
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
