[jdk8u-dev] RFR: 8315135: Memory leak in the native implementation of Pack200.Unpacker.unpack()

[Andrew John Hughes]

On Thu, 31 Aug 2023 22:30:35 GMT, Andrew John Hughes <andrew at openjdk.org> wrote:

>> Hi all,
>> 
>> This pull request contains a backport of commit [b77c161e](https://github.com/openjdk/jdk11u-dev/commit/b77c161e7509aa3b09ebf3e6b2b1490c0667bbdc) from the [openjdk/jdk11u-dev](https://git.openjdk.org/jdk11u-dev) repository.
>> 
>> The commit being backported was authored by Volker Simonis on 30 Aug 2023 and was reviewed by Christoph Langer and Thomas Stuefe.
>> 
>> The backport applies cleanly except for the usual 11/8 directory shuffling and the following cosmetic context change in `jni.cpp`:
>> 
>>> --- a/jdk/src/share/native/com/sun/java/util/jar/pack/jni.cpp
>>> +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/jni.cpp
>> 58c58
>> <    CHECK_EXCEPTION_RETURN_VALUE(uPtr, 0);
>> ---
>>>    CHECK_EXCEPTION_RETURN_VALUE(uPtr, NULL);
>> 
>> Thanks!
>
> Backport looks ok (though not clean due to the context difference)

> Thanks @gnu-andrew! Regarding the "clean" label, I was a little confused by the following line in the [OpenJDK Backports Wiki](https://wiki.openjdk.org/display/SKARA/Backports):
> 
> > A backport commit is considered clean if the changes in the original commit are identical to the changes in the backport commit. Note that only the changes have to be identical, not the changed lines.
> 
> And I actually still don't fully understand what exactly "**_Note that only the changes have to be identical, not the changed lines_**" means?

It's the first time I've seen that myself and I agree it's confusing. I think the intention may be to imply that it's clean if you make the same code changes even if they are in a different place in the file or even a different file. To me, that would be going a bit far.

The approach I've always used, even before we used GitHub and Skara, is that a backport is clean if it can be automated by tools i.e. if you apply the shuffle script, and then run `patch`, and it applies, it is clean. If you have to open a file and apply any hunks manually, that is not clean.

Even the former can be problematic if patch fuzzes the change into the wrong place, so I would be cautious even in those cases. That's why I actually quite like that 8u doesn't automatically add `clean` and patches do get a once over from a real human :) As someone has to approve anyway, I don't think it's really much extra work.

-------------

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/361#issuecomment-1702943787