[8u] RFR: 8218780: Update MUSCLE PCSC-Lite header files

Severin Gehwolf sgehwolf at redhat.com
Mon Sep 2 12:14:12 UTC 2019


Hi Andrew,

Thanks for the review!

On Wed, 2019-08-28 at 18:15 +0100, Andrew John Hughes wrote:
> On 26/08/2019 14:23, Severin Gehwolf wrote:
> > Hi,
> > 
> > Could I please get a review of this MUSCLE header files update in
> > OpenJDK 8u? I'd like to backport this bug as it's also going to be in
> > Oracle JDK 8u231 (equiv to OpenJDK 8u232) as well. The OpenJDK 11 patch
> > applies almost cleanly post path-unshuffelling. Changes which didn't
> > apply were a copyright header update in pcsc.c. I've omitted these.
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8218780
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8218780/jdk8/01/webrev/
> > JDK 11u changeset: https://hg.openjdk.java.net/jdk-updates/jdk11u-dev/rev/0bcc59a50c88
> > 
> > There is going to be a follow-up fix adding back COPYING via 
> > JDK-8226607 which I propose for backport to OpenJDK 8u as well.
> > 
> > Testing: smartcardio sanity and build on Linux x86_64 (Fedora 30 and RHEL 6)
> > 
> > Thanks to Aleksey Shipilev who helped test this patch.
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> > 
> 
> Most of this looks good. I was a little confused at first because the
> patch in your webrev looks quite different to the 11u changeset.
> However, once applied locally to the 8u repo, the diff between the two
> was as suggested and the PCSC library files (those in MUSCLE) were
> identical. I don't know what webrev did in creating that patch.
> 
> The bit I don't understand is why you've decided to drop the copyright
> header change on the floor. Just because the original in 8u has 2014,
> while the original in 11u had 2015, does not make the change inapplicable.

OK. I see. I wasn't sure how to deal with copyright year updates. I've
added the copyright update back. The patch is now identical to JDK 11u
(modulo differing copyright year base: 2014 - jdk 8 - vs. 2018 - jdk
11):
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8218780/jdk8/02/webrev


> A better alternative may actually be to backport JDK-8207233 [0] first
> which is a nice little cleanup patch. I suspect this patch would then
> apply cleanly as these PCSC files are rarely touched.
> 
> [0] https://bugs.openjdk.java.net/browse/JDK-8207233

Hmm, this seems overkill just to get the copyright hunk to apply. I'd
prefer to keep this dependency out of scope for this patch. While a
nice clean-up it's not without risk backporting that too. Thoughts?

Thanks,
Severin



More information about the jdk8u-dev mailing list