git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 5/8] status: print per-file porcelain v2 status data

From
Jeff King <peff@peff.net>
Date
Aug 5, 2016, 21:14 UTC
Message-ID
<20160805211434.54mtaw2cty4gaxsr@sigill.intra.peff.net>
In-Reply-To
<CAPc5daUr7OgFeefbwWLWFt3KW_9X-ijRmHa2oj0--zY4fqt82A@mail.gmail.com>
On Fri, Aug 05, 2016 at 02:09:48PM -0700, Junio C Hamano wrote:
Show 25 quoted lines
> On Fri, Aug 5, 2016 at 2:02 PM, Jeff King <peff@peff.net> wrote:
> > On Tue, Aug 02, 2016 at 10:12:14AM -0400, Jeff Hostetler wrote:
> >> +     switch (d->stagemask) {
> >> +     case 1: key = "DD"; break; /* both deleted */
> >> + ...
> >> +     case 7: key = "UU"; break; /* both modified */
> >> +     }
> >> [...]
> >> +     fprintf(s->fp, "%c %s %s %06o %06o %06o %06o %s %s %s %s%c",
> >> +                     unmerged_prefix, key, submodule_token,
> >
> > Coverity complains that "key" can be uninitialized here. I think it's
> > wrong, and just doesn't know that d->stagemask is constrained to 1-7.
> >
> > But perhaps it is worth adding a:
> >
> >   default:
> >         die("BUG: unhandled unmerged status %x", d->stagemask);
> >
> > to the end of the switch. That would shut up Coverity, and give us a
> > better indication if our constraint is violated.
> 
> This is pure curiosity but I wonder if Coverity shuts up if we
> instead switched on (d->stagemask & 07). Your "default: BUG"
> suggestion is what we should use as a real fix, of course.

I suspect yes. It's pretty clever about analyzing the flow and placing constraints on values (though in this case "& 07" includes "0", which is not handled in the switch).

Unfortunately it is hard for me to test a one-off, as running it locally is a complete pain. Stefan set it up long ago to pull "pu" and email out the results from their central servers, so I just scan those emails for things that look like real issues.

-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 16 of 31 in “status: V2 porcelain status”
  1. 0/8 status: V2 porcelain statusJeff Hostetler, Aug 2, 2016
  2. 2/8 status: cleanup API to wt_status_printJeff Hostetler, Aug 2, 2016
  3. Junio C HamanoAug 3, 2016
  4. Jeff HostetlerAug 4, 2016
  5. 3/8 status: support --porcelain[=<version>]Jeff Hostetler, Aug 2, 2016
  6. Junio C HamanoAug 3, 2016
  7. 8/8 status: tests for --porcelain=v2Jeff Hostetler, Aug 2, 2016
  8. Junio C HamanoAug 5, 2016
  9. Jeff HostetlerAug 5, 2016
  10. 6/8 status: print branch info with --porcelain=v2 --branchJeff Hostetler, Aug 2, 2016
  11. Junio C HamanoAug 5, 2016
  12. Jeff HostetlerAug 5, 2016
  13. 5/8 status: print per-file porcelain v2 status dataJeff Hostetler, Aug 2, 2016
  14. Jeff KingAug 5, 2016
  15. Junio C HamanoAug 5, 2016
  16. Jeff KingAug 5, 2016
  17. Junio C HamanoAug 5, 2016
  18. Stefan BellerAug 5, 2016
  19. Stefan BellerAug 5, 2016
  20. Jeff HostetlerAug 5, 2016
  21. 7/8 git-status.txt: describe --porcelain=v2 formatJeff Hostetler, Aug 2, 2016
  22. Junio C HamanoAug 5, 2016
  23. Jeff HostetlerAug 5, 2016
  24. 1/8 status: rename long-format print routinesJeff Hostetler, Aug 2, 2016
  25. Junio C HamanoAug 3, 2016
  26. Jeff HostetlerAug 4, 2016
  27. 4/8 status: per-file data collection for --porcelain=v2Jeff Hostetler, Aug 2, 2016
  28. Johannes SchindelinAug 3, 2016
  29. Junio C HamanoAug 3, 2016
  30. Johannes SchindelinAug 3, 2016
  31. Junio C HamanoAug 3, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.