You are right Junio! They are mutually exclusives and your last suggestion's patch is totally coherent, instead of mine. My attempt about this combination was driven by a missunderstanding of the command and the goal to find all ancestors' branches, sorted reverse-chronologicaly, for a given commit. I'm happy to have contributed to the git project by discovering the segfault. Happy, also, that together, we achieve a better fix than I done. Hope this would help all of us, as it will ensure a more stable use in our devs. Sincerely. On 22/04/2022 17:26, Junio C Hamano wrote: > "gdd via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> From: Gregory DAVID <gregory.david@xxxxxxxxx> >> >> 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. -- Gregory David Security Engineer https://www.p1sec.com
Attachment:
OpenPGP_0xA6DF37692E5DE2F7.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
