Re: [RFC] Re: Convert 'git blame' to parse_options()
On Tue, Jun 24, 2008 at 05:05:50PM +0000, Linus Torvalds wrote:
Show 22 quoted lines
>
>
> On Tue, 24 Jun 2008, Pierre Habouzit wrote:
> >
> > Actually this doesn't work because it may point into the strbuf that
> > will be invalidated later. Our sole option here is to leak some memory I
> > fear.
>
> I think leaking memory is ok (it's obviously going to be bounded by the
> size of the arguments you pass into a program), but I also think you can
> just change the option strings in place.
>
> Yeah, I know - it's impolite, and we even marked things "const char", but
> "const" in C is just a politeness thing, we can just choose to have a
> function with a big comment that changes the string anyway. We'll have to
> make sure that the few places that actually create argument strings by
> hand (ie not from the ones supplied by a real "execve()") not do them so
> that they need splitting (but no current ones would need to, obviously,
> since splitting the argumens isn't even supported yet).
>
> Or, if people hate that, just leak a few malloc'ed areas. That's arguably
> the more straightforward way.
The problem is that we sometimes feed the option parser with hand
crafted const char *[] where strings are indeed in rodata, and well,
changing things is not only impolite, it tends to SIG{BUS,SEGV} ;) But I think too that leaking is an option. git rev-parse --parseopt
already leaks in the same ways e.g.
Though for the win32 port where fork is replaced with threads, well,
it may cause some issues, so I was reluctant wrt them. Of course it's
unlikely that it will cause problems, but one never knows ;)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
Pierre Habouzit <madcoder@debian.org>