From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Junio C Hamano <gitster@pobox.com>,
Johan Herland <johan@herland.net>,
Johannes Schindelin <johannes.schindelin@gmx.de>,
git@vger.kernel.org, Stefan Beller <sbeller@google.com>,
Lars Schneider <larsxschneider@gmail.com>
Subject: Re: [PATCH 00/12] Clean up notes-related code around `load_subtree()`
Date: Sun, 10 Sep 2017 03:39:28 -0400 [thread overview]
Message-ID: <20170910073928.ys4nbap76tmiurjh@sigill.intra.peff.net> (raw)
In-Reply-To: <2b7c0053-bf7a-fbdd-3cf9-39b5d9a962c3@alum.mit.edu>
On Sun, Sep 10, 2017 at 06:45:08AM +0200, Michael Haggerty wrote:
> > So nothing to see here, but since I spent 20 minutes scratching my head
> > (and I know others look at Coverity output and may scratch their heads
> > too), I thought it was worth writing up. And also if I'm wrong, it would
> > be good to know. ;)
>
> Thanks for looking into this. I agree with your analysis.
>
> I wonder whether it is the factor of two between path lengths and byte
> lengths that is confusing Coverity. Perhaps the patch below would help.
> It requires an extra, superfluous, check, but perhaps makes the code a
> tad more readable. I'm neutral on whether we would want to make the change.
Yeah, I do agree that it makes the code's assumptions a bit easier to
follow.
> Is there a way to ask Coverity whether a hypothetical change would
> remove the warning, short of merging the change to master?
You can download and run the build portion of the coverity tools
yourself. IIRC, that pushes the build up to their servers which then do
the analysis (you can make your own "project", or use the existing "git"
project -- I checked and you are already listed as an admin). I recall
it being a minor pain to get it set up, but not too bad.
Stefan runs it against "pu" on a regular basis, which is where the
emailed results come from. So just having Junio merge it to "pu" would
be enough to get results.
I noticed that they now have some GitHub/Travis integration:
https://scan.coverity.com/github
I'm not sure if that is new, or if we just didn't notice it before. ;)
But that probably makes more sense to use than ad-hoc uploading (and
maybe it would make it easy for you to test personal branches, too).
-Peff
next prev parent reply other threads:[~2017-09-10 7:39 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-26 8:28 [PATCH 00/12] Clean up notes-related code around `load_subtree()` Michael Haggerty
2017-08-26 8:28 ` [PATCH 01/12] notes: make GET_NIBBLE macro more robust Michael Haggerty
2017-08-26 8:28 ` [PATCH 02/12] load_subtree(): remove unnecessary conditional Michael Haggerty
2017-08-26 16:38 ` Junio C Hamano
2017-08-27 6:37 ` Michael Haggerty
2017-08-28 6:55 ` Michael Haggerty
2017-09-01 21:53 ` Junio C Hamano
2017-08-26 8:28 ` [PATCH 03/12] load_subtree(): reduce the scope of some local variables Michael Haggerty
2017-08-26 8:28 ` [PATCH 04/12] load_subtree(): fix incorrect comment Michael Haggerty
2017-08-26 8:28 ` [PATCH 05/12] load_subtree(): separate logic for internal vs. terminal entries Michael Haggerty
2017-08-26 8:28 ` [PATCH 06/12] load_subtree(): check earlier whether an internal node is a tree entry Michael Haggerty
2017-08-26 8:28 ` [PATCH 07/12] load_subtree(): only consider blobs to be potential notes Michael Haggerty
2017-08-26 8:28 ` [PATCH 08/12] get_oid_hex_segment(): return 0 on success Michael Haggerty
2017-08-26 8:28 ` [PATCH 09/12] load_subtree(): combine some common code Michael Haggerty
2017-08-26 8:28 ` [PATCH 10/12] get_oid_hex_segment(): don't pad the rest of `oid` Michael Haggerty
2017-08-26 8:28 ` [PATCH 11/12] hex_to_bytes(): simpler replacement for `get_oid_hex_segment()` Michael Haggerty
2017-08-26 8:28 ` [PATCH 12/12] load_subtree(): declare some variables to be `size_t` Michael Haggerty
2017-08-26 23:36 ` [PATCH 00/12] Clean up notes-related code around `load_subtree()` Johan Herland
2017-09-09 10:31 ` Jeff King
2017-09-10 4:45 ` Michael Haggerty
2017-09-10 7:39 ` Jeff King [this message]
2017-09-12 6:47 ` Michael Haggerty
2017-09-12 11:55 ` Lars Schneider
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170910073928.ys4nbap76tmiurjh@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johan@herland.net \
--cc=johannes.schindelin@gmx.de \
--cc=larsxschneider@gmail.com \
--cc=mhagger@alum.mit.edu \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).