This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Fix BZ#16374 -- don't use mmap for FILE buffers


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]