Re: [PATCH v2 1/3] help: redirect to aliased commands for "git cmd --help"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 03, 2018 at 08:24:14AM +0200, Rasmus Villemoes wrote:

> > The comment probably needs to spell out that exclude_guides
> > is the same as your "we were invoked as...".
> 
> Will do. That will also make the string --exclude-guides (i.e., with a
> dash) appear in the comment, making it more likely to be found should
> anyone change when and how --exclude-guides is implied.

OK. I can live with that.

> > So we split only to find argv[0] here. But then we don't return it. That
> > works because the split is done in place, meaning we must have inserted
> > a NUL in alias. That's sufficiently subtle that it might be worth
> > spelling it out in a comment.
> 
> OK, I actually had precisely
> 
> +		/*
> +		 * We use split_cmdline() to get the first word of the
> +		 * alias, to ensure that we use the same rules as when
> +		 * the alias is actually used. split_cmdline()
> +		 * modifies alias in-place.
> +		 */
> 
> in v1, but thought it might be overly verbose. I'll put it back in.

:) That's perfect.

> > We don't need to free alias here as we do above, because we're passing
> > it back. We should free argv, though, I think (not its elements, just
> > the array itself).
> 
> Yeah, I thought about this, and removing free(argv) was the last thing I
> did before sending v1 - because we were going to leak alias anyway. I'm
> happy to put it back in, along with...

Thanks. I think this is different than "alias" because we really do leak
it _here_, whereas alias lives on and can be UNLEAKed later.

> > Unfortunately the caller is going to leak our returned "alias", [...] I think it may be OK to overlook
> > that and just UNLEAK() it in cmd_help().
> 
> ...this. Except I'd rather do the UNLEAK in check_git_cmd (the
> documentation does say "only from cmd_* functions or their direct
> helpers") to make it a more targeted annotation.

Yeah, I think that's fine. Thanks!

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux