Jeff King <peff@xxxxxxxx> writes:
> On Mon, Aug 19, 2013 at 09:03:21PM +0100, Philip Oakley wrote:
>
>> In case other readers don't have a .xlsx reader here is Rick's list
>> in plain text (may be white space damaged).
>>
>> I expect some will be false positives, and some will just be being
>> too cautious.
>>
>> [...]
>>
>> description resourceFilePath fileName lineNumber
>> nullPointer(CppCheck) \git-master\builtin\add.c add.c 286
>
> Hm. That code in v1.8.3.4 reads:
>
> if (pathspec)
> while (pathspec[pc])
> pc++;
>
> What's the problem? If pathspec is not properly terminated, we can run
> off the end, but I do see anything to indicate that is the case. What
> does the "nullPointer" check mean here?
>
>> wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c
>> fetch.c 588
>
> Line 588 does not have formatted I/O at all. Are these line numbers
> somehow not matching what I have in v1.8.3.4?
>
>> nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
>> 144
>
> This one looks like:
>
> if (tag && *tag && show_valid_bit &&
> (ce->ce_flags & CE_VALID)) {
> static char alttag[4];
> memcpy(alttag, tag, 3);
> if (isalpha(tag[0]))
>
> where the final line is 144. But we have explicitly checked that tag is not
> NULL...
>
>> doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275
>
> This one looks like:
>
> if (...) {
> free(buf);
> die(...);
> }
> ...
> free(buf);
>
> which might look like a double free if you do not know that die() will
> never return (it is properly annotated for gcc, but I don't know whether
> CppCheck understands such things).
>
> So out of the 4 entries I investigated, none of them looks like an
> actual problem. But I'm not even sure I am looking at the right place;
> these don't even seem like things that would cause a false positive in a
> static analyzer.
And the ones I picked at random looks totally bogus, too.
uninitvar(CppCheck) \git-master\notes.c notes.c 805
uninitvar(CppCheck) \git-master\notes.c notes.c 805
That is
int combine_notes_concatenate(unsigned char *cur_sha1,
const unsigned char *new_sha1)
{
char *cur_msg = NULL, *new_msg = NULL, *buf;
unsigned long cur_len, new_len, buf_len;
enum object_type cur_type, new_type;
int ret;
/* read in both note blob objects */
if (!is_null_sha1(new_sha1))
new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
805: if (!new_msg || !new_len || new_type != OBJ_BLOB) {
free(new_msg);
return 0;
}
new_msg starts out to be NULL, if we did not run read_sha1_file(),
it will still be NULL and "if()" will not look at new_len/new_type
so their being uninitialized does not matter. If we did run
read_sha1_file(), and if it returns a non-NULL new_msg, both of
these are filled. If read_sha1_file() returns a NULL new_msg, again
the other two does not matter.
In short, the analyzer seems to be giving useless noise for this
one.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html