This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [patch] Fix BZ#16374 -- don't use mmap for FILE buffers
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: Paul Pluzhnikov <ppluzhnikov at google dot com>, GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Tue, 17 Feb 2015 09:52:35 -0500
- Subject: Re: [patch] Fix BZ#16374 -- don't use mmap for FILE buffers
- Authentication-results: sourceware.org; auth=none
- References: <CALoOobNomWyxd9Oz3=kHq0vyBpmfxSyj_cFBxyahCJSs1cZBzQ at mail dot gmail dot com> <54E236D9 dot 3010807 at redhat dot com> <CALoOobPVr0PAkzDtQbXXGP7Vyu-Ls+V-1JGXpi8yBbyryqYf5g at mail dot gmail dot com> <54E23F28 dot 6040304 at redhat dot com> <alpine dot DEB dot 2 dot 10 dot 1502162204510 dot 10686 at digraph dot polyomino dot org dot uk>
On 02/16/2015 05:06 PM, Joseph Myers wrote:
> On Mon, 16 Feb 2015, Carlos O'Donell wrote:
>
>> On 02/16/2015 01:51 PM, Paul Pluzhnikov wrote:
>>> On Mon, Feb 16, 2015 at 10:28 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>
>>>> This patch is not OK as-is, and needs to be split into multiple patches.
>>>>
>>>> One patch for the calloc/free changes, and another with much much much
>>>> more detail for the semantic changes that fix input-only unclean buffers.
>>>
>>> The first patch will break 3 (mtrace) tests, and the second will then
>>> fix them. Is that ok?
>>
>> Yes, post them as 0/2 (descriptions), 1/2 (conversion), and 2/2 (bug fix).
>
> No, we want bisectability and should seek to avoid any commits on master
> that introduce test failures.
I agree, but that doesn't have to impact how you post the patches, only
how you commit them. Paul can ask for both to be committed as one commit,
but split them for review. In patchwork he can make a group out of them
and we can make it a habit to always commit a group as a single commit.
Similarly for patches that apply on top of eachother, it's obvious that
intermediate results likely won't build, so you have to commit them as
a single group of patches in one commit.
For the purposes of review I have *some* idea of what is and is not part
of the fix, but Paul needs to make that explicit by splitting the patches
(but not necessarily the commits).
> If the bug fixes can go first (even if the bugs being fixed are latent at
> present), then do them before the conversion, not after. If they can't go
> first then they may need to go in the same patch as the conversion.
Agreed.
> (If there weren't existing tests involved then splitting in your order
> would work, with patch 2/2 being the one adding the new tests.)
Cheers,
Carlos.