Re: What's cooking in git.git (Jun 2012, #02; Wed, 6)

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jun 7, 2012, 00:39 UTC
Message-ID: <20120607003904.GB4065@burratino>
In-Reply-To: <CAFfmPPOWZz8JF_BVKvnAVp0VUDzxPiVSqjG7ATPx3CVztDF=cw@mail.gmail.com>

Hi,

David Michael Barr wrote:

> On Thu, Jun 7, 2012 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

>> * db/vcs-svn (2012-06-01) 6 commits
>>  - vcs-svn: drop no-op reset methods
>>  - vcs-svn: fix signedness warnings
>>  - vcs-svn: prefer strstr over memmem
>>  - vcs-svn: prefer constcmp to prefixcmp
>>  - vcs-svn: simplify cleanup in apply_one_window()
>>  - vcs-svn: fix clang-analyzer error
>>
>> I do not know the doneness of this series that came out of the
>> blue.  Are people involved in vcs-svn happy with this series?
>
> At the end of this series, vcs-svn is static-analyzer clean when
> merged into svn-dump-fast-export.

Thanks for the cc. I generally like the code and generally dislike the descriptions which amount to "appease the static analyzer" and don't reflect the thought you actually put into the patches, which sets a bad precedent. I was planning to munge the descriptions, ask you to look it over, and then ask Junio to pull.

The signedness patch still leaves me worried: for example, where len is of type (size_t) and delta_len of type (off_t *),

	-       if (len > *delta_len ||
	+       if ((off_t) len > *delta_len ||

changes the meaning for the worse if len is very large on a system (think: 64-bit PC) where size_t and off_t have the same width. It's exploitable. :(

But I like the spirit of the patches.

The "drop no-op reset methods" patch is unrisky and could go in directly.

Jonathan