Re: [PATCH] show-branch: fix SEGFAULT on both --current & --reflog

> "gdd via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Gregory DAVID <gregory.david@p1sec.com>
>>
>> If run `show-branch` with `--current` and `--reflog` simultaneously, a
>> SEGFAULT appears.
> 
> Thanks for noticing.  As I said elsewhere, "--current" was invented
> for the sole purpose of being used together with branches and
> individual commits, not in "--reflog" or "--merge-base" modes.
> 
> And as I also said elsewhere, I do not think of a good reason why a
> user would want to use these two together.
> 
> Can you tell us a bit more about what you were trying to achieve
> when you used them together?
> 
> While waiting for your (and others) valid use cases I probably have
> missed (I said "do not think of" above, not "I think there cannot
> be"), let's speculate what sensible meaning the combination could
> have.
> 
> As is clear from an existing error when two branches are given:
> 
>     $ git show-branch --reflog master maint
>     fatal: --reflog option needs one branch name
> 
> the "--reflog" mode is not even prepared to work with more than one
> branch.  It is to show reflog entries taken from one branch (it
> could be HEAD)'s reflog.
> 
> But "--current" is about "Among the branches I listed on the command
> line, the current branch may or may not be included. If not, please
> pretend as if I had the current branch there, too".
> 
> So, if we said
> 
>     $ git show-branch --reflog --current maint
> 
> while we are on 'master' branch, that is the same as saying
> 
>     $ git show-branch --reflog master maint
> 
> which should get a usage error, and if we are on 'maint' branch,
> then maint is already there, so there is no point in giving
> '--current' to begin with.
> 
> Because
> 
>     $ git show-branch --reflog
> 
> defaults to showing the reflog entries from current branch,
> 
>     $ git show-branch --reflog --current 
> 
> hoping that it would show the reflog entries of the current branch
> is indeed a plausible interpretation, but even in this case, it is
> not necessary to give "--current".
> 
> So, unless there is a reason why it makes sense to enumerate recent
> reflog entries from a branch *and* the tip of the current branch at
> the same time, I am very much inclined to make it clear that
> "--reflog" and "--current" are mutually incompatible by making it an
> error to give both.
> 
> In addition, we _could_ allow a command line with "--reflog --current"
> and nothing else on it, and ignore "--current" only in that case, to
> "support" the "plausible interpretation" above, but I do not think
> it is worth it.
> 
>> It seems that it has been introduced in: Commit 1aa68d6735
>> (show-branch: --current includes the current branch., 2006-01-11)
> 
> Yes, the commit should have noticed the invalid combination of
> options were given and errored out.  Since omission of such a check
> lead to a segfaulting bug without producing any useful output, it
> is safe to make it an error to give these options at the same time.
> 
> Thanks.