RFR: 8u: 8076475: Misuses of strncpy/strncat

[Andrew Hughes]

On 03/04/2020 10:18, Andrew Haley wrote:
> On 4/3/20 4:44 AM, Andrew Hughes wrote:
>>
>> On 02/04/2020 09:36, Andrew Haley wrote:
>>> On 4/1/20 6:54 PM, Andrew Hughes wrote:
>>>> * src/share/vm/runtime/os.{c,h}pp
>>>>   If we're going to add strdup_check_oom, I think this should be done by
>>>> backporting JDK-6424123 [1] so it's used consistently throughout the
>>>> codebase rather than just the two call sites in this patch.
>>>
>>> I strongly disagree that we should import such a patch into JDK 8 for
>>> this reason.
>>>
>>> It's a judgement call, of course. We have two desiderata here: those
>>> of consistently backporting patches and minimizing changes. As JDK 8
>>> matures, minimizing change has become more and more important, and it's
>>> perfectly OK to snip small fragments of patches such as this in order
>>> to minimize change.
>>>
>>> Of course I'm aware of the other side of the argument, but I don't
>>> think it's as important.
>>
>> And I strongly disagree with turning 8u into a unmaintainable mass of
>> partial chunks of code from various changesets.
> 
> I have no intention of doing that, but pulling in large and otherwise
> unnecessary changesets for single functions should not be done in a
> release of a mature product.
> 
> Here is my reasoning. The need for stability and the minimization of
> churn increases over time. People use JDK 8 because they want that
> stability, and every patch carries some risk. So, ecause it is now a
> mature piece of software, patches should be minimal where they
> reasonably can. The practice of, by default, pulling in dependencies
> from later releases is too risky: they might well be broken in ways
> that the old version wasn't, and they may depend on other changes in
> the recent code base.
> 
> Now, that doesn't mean that such dependent changesets must *never* be
> pulled in, especially when they are needed by several backports. But
> if dependency changesets substantially multiply the size of a change,
> as they do in this case, this should not be done.
> 
> I am aware that this does not ease the life of the maintainers, but
> that isn't the goal. Stability is.

It's not about making life easier for maintainers; reviewing and
approving more patches means the opposite. My concern is that the more
8u diverges from later versions, and uses its own bespoke version of
fixes, the greater the knowledge and experience needed for someone to
work on it. We're already struggling to keep up as it is.

Each case has to be considered separately, according to risk and the
changes involved. It's not a guarantee of stability to just not make
changes. If it leads to the changes that are made to 8u being unique to
that codebase, and not as widely tested as something that has been in
later versions for years, we will introduce more 8u-specific bugs. In
short, there is no easy answer and no one size fits all solution.

> 
> We need to discuss this issue some more, with a view to producing some
> updated guidelines.
> 
>> In this case, it would lead to a strdup failure being handled
>> differently, depending on where in the code it occurred.
>> The alternative would be to use os::strdup directly and avoid
>> introducing this function. This was how JDK-8155968 already handled
>> this issue.
> 
> That sounds like an excellent idea. I'll prepare a new patch.
> 

New patch looks good. I've approved and pushed it, so it will be part of
jdk8u262-b04.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222