Derrick Stolee <sto...@gmail.com> writes:
>> +++ b/Documentation/pretty-formats.txt
>> @@ -134,6 +134,8 @@ The placeholders are:
>> - '%cI': committer date, strict ISO 8601 format
>> - '%d': ref names, like the --decorate option of linkgit:git-log[1]
>> - '%D': ref names without the " (", ")" wrapping.
>> +- '%S': ref name given on the command line by which the commit was reached
>> + (like `git log --source`), only works with `git log`
>
> This "only works with `git log`" made me think about what would happen
> with `git rev-list --pretty=format:"%h %S"` and the answer (on my
> machine) was a segfault.
That's a bad one X-<.
>> + slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
>> + if (slot && *slot) {
> I'm not sure this check for 'slot' being non-null is necessary, as we
> would already get a failure in the commit-slab code (for
> revision_sources_at()) if the slab is not initialized.
>> + strbuf_addstr(sb, *slot);
>> + return 1;
>> + } else {
>> + die(_("failed to get info for %%S"));
>
> Here, you die() when you fail to get a slot but above you return 0
> when the sources are not initialized.
>
> I don't see another use of die() in this method. Is that the right way
> to handle failure here? (I'm legitimately asking because I have
> over-used 'die()' in the past and am still unclear on when it is
> appropriate.)
This is definitely a bad one, too. If '%d' cannot find decoration,
it would not "die".
Thanks.
