Re: Potential Null Pointer Dereference detected by static analysis tool

> >   1. What should happen when traversing from HEAD does not find the blob
> >      in question? Right now we print a blank line, which is...weird.
> 
> Weird indeed.
> 
> >      Probably we should either print nothing, or return an error.
> 
> The latter, consistent with "git describe <commit-ish>".

> > If we return an error, should we respect --always?
> 
> The documentation says "git describe <blob>" takes no options.  It could
> learn some, of course.  But does it have to?  Perhaps better keep that
> thing contained.

> >   2. When we are on an unborn branch, we print a confusing message:
> > 
> >        $ git init
> >        $ git commit --allow-empty -m foo
> >        $ git tag foo
> >        $ git symbolic-ref HEAD refs/heads/unborn
> >        $ git describe $(echo blob | git hash-object -w --stdin)
> >        fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.
> >        Use '--' to separate paths from revisions, like this:
> >        'git <command> [<revision>...] -- [<file>...]'
> > 
> >      We should probably resolve HEAD ourselves and either bail with an
> >      empty output or an error (depending on what we do for (1) above).
> 
> It already is an error, just needs a better message.  It should still
> report an error even if we were to stick with showing blank lines for
> unrelated blobs.

> >   3. When we do traverse, if process_object() sees that we didn't find a
> >      commit, we should detect that and either return an empty result or
> >      an error (again, depending on the behavior of (2) above). This is
> >      done by checking is_null_oid(&pcd->current_commit) there.
> 
> OK, ending the search right there might be the best option.  Traversing
> deeper into the forest that we then know to be cursed would be the
> unappealing alternative.

>   5. When process_object() has a commit, but it is indescribable, it
>      shows an error:
> 
>      $ git describe 5afbe6da1d6ab0b8939343166636b828e561bf35
>      fatal: No tags can describe '3b681e255cd8577a77983958ef7f566b05806cd0'.
>      Try --always, or create some tags.
> 
>      It's not immediately clear that the reported hash belongs to the
>      found commit.  And that suggestion to try --always is misleading,
>      as "git describe <blob>" takes no options according to the
>      documentation.  I'm not sure I like it in general -- can't tell
>      if the command is being snarky with me.

> > So it all depends on what to do with (1), and for a feature that IMHO
> > should not even exist in the first place, I had trouble summoning the
> > will-power to make this 4-patch series.
> 
> 644eb60bd0 (builtin/describe.c: describe a blob, 2017-11-15) and
> 15af58c1ad (diffcore: add a pickaxe option to find a specific blob,
> 2018-01-04) confuse me; the latter's commit message sounds like the
> former wasn't (supposed to be?) merged.
> 
> I think the issues you listed are independent, though.  Or what's wrong
> with this demo that addresses point 3 in process_object() and 1 in
> describe_blob().  If we want a blank line for 1 then we apply only
> the first hunk.  Or am I missing something?