Re: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 02/04/2019 13:54, Baesken, Matthias wrote: Hi Alan, here is a new webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.3/webrev/ re-formatted the comment in java_md.cto avoid VERY LONG lines adjusted test/jdk/tools/launcher/Arrrghs.java Best regards, Matthias Using String.repeat instead of 340+ character line is much better :-) So yes, I think this version looks good. -Alan
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello, is the latest revision fine with you ?
May I add you as a reviewer ?
Thanks, Matthias
> -Original Message-
> From: Baesken, Matthias
> Sent: Dienstag, 2. April 2019 14:55
> To: 'Alan Bateman'
> Cc: core-libs-dev@openjdk.java.net
> Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> Hi Alan, here is a new webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.3/webrev/
>
> re-formatted the comment in java_md.cto avoid VERY LONG lines
> adjusted test/jdk/tools/launcher/Arrrghs.java
>
>
> Best regards, Matthias
>
>
>
> > -Original Message-
> > From: Alan Bateman
> > Sent: Samstag, 30. März 2019 10:04
> > To: Baesken, Matthias
> > Cc: core-libs-dev@openjdk.java.net
> > Subject: Re: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > On 29/03/2019 12:36, Baesken, Matthias wrote:
> > > Thanks. Alan, are you fine with the current webrev, if yes may I add you
> as
> > reviewer ?
> > >
> > The update to java_md.c looks okay, I probably would re-format the
> > comment to void the line longs but it is otherwise okay.
> >
> > I think the new test needs a of bit clean-up, L490 is the main issue.
> > One suggest is to rename pLong to longPath and build up ots value in a
> > loop to avoid having a 340+ character line? The other variables names
> > are very strange too but I can you can eliminate some of them by
> > changing L491-492 to:
> > Path jarPath = Files.createDirectories(longPah).resolve("foo.jar");
> >
> > -Alan
> >
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Alan, here is a new webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.3/webrev/
re-formatted the comment in java_md.cto avoid VERY LONG lines
adjusted test/jdk/tools/launcher/Arrrghs.java
Best regards, Matthias
> -Original Message-
> From: Alan Bateman
> Sent: Samstag, 30. März 2019 10:04
> To: Baesken, Matthias
> Cc: core-libs-dev@openjdk.java.net
> Subject: Re: RFR: 8218547: Simplify JLI_Open on Windows in native code
> (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> On 29/03/2019 12:36, Baesken, Matthias wrote:
> > Thanks. Alan, are you fine with the current webrev, if yes may I add you
> > as
> reviewer ?
> >
> The update to java_md.c looks okay, I probably would re-format the
> comment to void the line longs but it is otherwise okay.
>
> I think the new test needs a of bit clean-up, L490 is the main issue.
> One suggest is to rename pLong to longPath and build up ots value in a
> loop to avoid having a 340+ character line? The other variables names
> are very strange too but I can you can eliminate some of them by
> changing L491-492 to:
> Path jarPath = Files.createDirectories(longPah).resolve("foo.jar");
>
> -Alan
>
Re: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 29/03/2019 12:36, Baesken, Matthias wrote:
Thanks. Alan, are you fine with the current webrev, if yes may I add you as
reviewer ?
The update to java_md.c looks okay, I probably would re-format the
comment to void the line longs but it is otherwise okay.
I think the new test needs a of bit clean-up, L490 is the main issue.
One suggest is to rename pLong to longPath and build up ots value in a
loop to avoid having a 340+ character line? The other variables names
are very strange too but I can you can eliminate some of them by
changing L491-492 to:
Path jarPath = Files.createDirectories(longPah).resolve("foo.jar");
-Alan
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Thanks. Alan, are you fine with the current webrev, if yes may I add you as
reviewer ?
Best regards, Matthias
> -Original Message-
> From: Langer, Christoph
> Sent: Donnerstag, 28. März 2019 12:41
> To: Baesken, Matthias
> Cc: core-libs-dev@openjdk.java.net; Alan Bateman
>
> Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> Hi Matthias,
>
> this looks good to me now. Let's wait for another review then.
>
> Best regards
> Christoph
>
> > -Original Message-
> > From: Baesken, Matthias
> > Sent: Donnerstag, 28. März 2019 12:39
> > To: Langer, Christoph
> > Cc: core-libs-dev@openjdk.java.net; Alan Bateman
> >
> > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > Hello here is another webrev , I adjusted
> > test/jdk/tools/launcher/Arrrghs.java a bit taking your suggestions into
> > account .
> > Can I have a second review please ?
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.2/webrev/
> >
> > > As for the test, I think you could also check a jar in a short path
> >
> > There exist already quite a few tests using "-jar some.jar" with
> > "normal"
> /
> > shorter paths in test/jdk/tools/launcher/Arrrghs.java.
> >
> >
> > Regards, Matthias
> >
> >
> > > -Original Message-
> > > From: Langer, Christoph
> > > Sent: Donnerstag, 28. März 2019 11:59
> > > To: Baesken, Matthias
> > > Cc: core-libs-dev@openjdk.java.net; Alan Bateman
> > >
> > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > > parse_manifest.c on windows
> > >
> > > Hi Matthias,
> > >
> > > the change to src/java.base/windows/native/libjli/java_md.c looks good
> to
> > > me now.
> > >
> > > As for the test, I think you could also check a jar in a short path to
> > > exercise
> > > both cases in JLI_Open.
> > >
> > > And a few nits:
> > > Line 506: better do:
> > > Path pelp = pcreated.resolve(elp.jar);
> > >
> > > Line 507:
> > > 2 spaces are one too much after elp
> > >
> > > Line 508:
> > > Insert 2 additional spaces in the source code string, a space between "){"
> > and
> > > another in the end after the ; of the System.out.println statement, like
> this:
> > > createJar(elp, new File("Foo"), "public static void main(String[] args) {
> > > System.out.println(\"Hello from ELP\"); }");
> > >
> > > You could also do a little cleanup: Remove method private static
> > > String
> > > removeExtraQuotes(String in) { from line 64. It seems, it is not needed.
> > >
> > > Best regards
> > > Christoph
> > >
> > > > -Original Message-
> > > > From: Baesken, Matthias
> > > > Sent: Mittwoch, 27. März 2019 18:05
> > > > To: Langer, Christoph
> > > > Cc: core-libs-dev@openjdk.java.net; Alan Bateman
> > > >
> > > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > > > parse_manifest.c on windows
> > > >
> > > > Hello could you please look into it ?
> > > >
> > > > Best regards, Matthias
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Baesken, Matthias
> > > > > Sent: Montag, 25. März 2019 18:05
> > > > > To: Langer, Christoph
> > > > > Cc: 'core-libs-dev@openjdk.java.net' > d...@openjdk.java.net>;
> > > > > 'Alan Bateman'
> > > > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native
> code
> > > > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > > > > parse_manifest.c on windows
> > > > >
> > > > > Hello here is an updated webrev :
> > > > >
> > > > >
> > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.1/webrev/
> > > > >
> > > > >
> > > > >
> > > > > > > In JLI_Open(const char* name, int flags), you should remove ret
> > and
> > > > only
> > > > > > > use fd, I think.
> > > > >
> > > > >
> > > > > I removed ret and adjusted some comments.
> > > > >
> > > > > Additionally I added a test that uses (on Windows) JLI_Open on
> > > > > a
> > jar
> > > > file
> > > > > with a "long" path (> 260 chars).
> > > > > [ JLI_Open is currently called from parse_manifest.c with
> > > > > jarfile as
> > > > > parameter ]
> > > > >
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Matthias,
this looks good to me now. Let's wait for another review then.
Best regards
Christoph
> -Original Message-
> From: Baesken, Matthias
> Sent: Donnerstag, 28. März 2019 12:39
> To: Langer, Christoph
> Cc: core-libs-dev@openjdk.java.net; Alan Bateman
>
> Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> Hello here is another webrev , I adjusted
> test/jdk/tools/launcher/Arrrghs.java a bit taking your suggestions into
> account .
> Can I have a second review please ?
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.2/webrev/
>
> > As for the test, I think you could also check a jar in a short path
>
> There exist already quite a few tests using "-jar some.jar" with
> "normal" /
> shorter paths in test/jdk/tools/launcher/Arrrghs.java.
>
>
> Regards, Matthias
>
>
> > -Original Message-
> > From: Langer, Christoph
> > Sent: Donnerstag, 28. März 2019 11:59
> > To: Baesken, Matthias
> > Cc: core-libs-dev@openjdk.java.net; Alan Bateman
> >
> > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > Hi Matthias,
> >
> > the change to src/java.base/windows/native/libjli/java_md.c looks good to
> > me now.
> >
> > As for the test, I think you could also check a jar in a short path to
> > exercise
> > both cases in JLI_Open.
> >
> > And a few nits:
> > Line 506: better do:
> > Path pelp = pcreated.resolve(elp.jar);
> >
> > Line 507:
> > 2 spaces are one too much after elp
> >
> > Line 508:
> > Insert 2 additional spaces in the source code string, a space between "){"
> and
> > another in the end after the ; of the System.out.println statement, like
> > this:
> > createJar(elp, new File("Foo"), "public static void main(String[] args) {
> > System.out.println(\"Hello from ELP\"); }");
> >
> > You could also do a little cleanup: Remove method private static String
> > removeExtraQuotes(String in) { from line 64. It seems, it is not needed.
> >
> > Best regards
> > Christoph
> >
> > > -Original Message-
> > > From: Baesken, Matthias
> > > Sent: Mittwoch, 27. März 2019 18:05
> > > To: Langer, Christoph
> > > Cc: core-libs-dev@openjdk.java.net; Alan Bateman
> > >
> > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > > parse_manifest.c on windows
> > >
> > > Hello could you please look into it ?
> > >
> > > Best regards, Matthias
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Baesken, Matthias
> > > > Sent: Montag, 25. März 2019 18:05
> > > > To: Langer, Christoph
> > > > Cc: 'core-libs-dev@openjdk.java.net' d...@openjdk.java.net>;
> > > > 'Alan Bateman'
> > > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > > > parse_manifest.c on windows
> > > >
> > > > Hello here is an updated webrev :
> > > >
> > > >
> > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.1/webrev/
> > > >
> > > >
> > > >
> > > > > > In JLI_Open(const char* name, int flags), you should remove ret
> and
> > > only
> > > > > > use fd, I think.
> > > >
> > > >
> > > > I removed ret and adjusted some comments.
> > > >
> > > > Additionally I added a test that uses (on Windows) JLI_Open on a
> jar
> > > file
> > > > with a "long" path (> 260 chars).
> > > > [ JLI_Open is currently called from parse_manifest.c with jarfile
> > > > as
> > > > parameter ]
> > > >
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello here is another webrev , I adjusted
test/jdk/tools/launcher/Arrrghs.java a bit taking your suggestions into
account .
Can I have a second review please ?
http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.2/webrev/
> As for the test, I think you could also check a jar in a short path
There exist already quite a few tests using "-jar some.jar" with "normal"
/ shorter paths in test/jdk/tools/launcher/Arrrghs.java.
Regards, Matthias
> -Original Message-
> From: Langer, Christoph
> Sent: Donnerstag, 28. März 2019 11:59
> To: Baesken, Matthias
> Cc: core-libs-dev@openjdk.java.net; Alan Bateman
>
> Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> Hi Matthias,
>
> the change to src/java.base/windows/native/libjli/java_md.c looks good to
> me now.
>
> As for the test, I think you could also check a jar in a short path to
> exercise
> both cases in JLI_Open.
>
> And a few nits:
> Line 506: better do:
> Path pelp = pcreated.resolve(elp.jar);
>
> Line 507:
> 2 spaces are one too much after elp
>
> Line 508:
> Insert 2 additional spaces in the source code string, a space between "){" and
> another in the end after the ; of the System.out.println statement, like this:
> createJar(elp, new File("Foo"), "public static void main(String[] args) {
> System.out.println(\"Hello from ELP\"); }");
>
> You could also do a little cleanup: Remove method private static String
> removeExtraQuotes(String in) { from line 64. It seems, it is not needed.
>
> Best regards
> Christoph
>
> > -Original Message-
> > From: Baesken, Matthias
> > Sent: Mittwoch, 27. März 2019 18:05
> > To: Langer, Christoph
> > Cc: core-libs-dev@openjdk.java.net; Alan Bateman
> >
> > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > Hello could you please look into it ?
> >
> > Best regards, Matthias
> >
> >
> >
> > > -Original Message-
> > > From: Baesken, Matthias
> > > Sent: Montag, 25. März 2019 18:05
> > > To: Langer, Christoph
> > > Cc: 'core-libs-dev@openjdk.java.net' ;
> > > 'Alan Bateman'
> > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > > parse_manifest.c on windows
> > >
> > > Hello here is an updated webrev :
> > >
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.1/webrev/
> > >
> > >
> > >
> > > > > In JLI_Open(const char* name, int flags), you should remove ret and
> > only
> > > > > use fd, I think.
> > >
> > >
> > > I removed ret and adjusted some comments.
> > >
> > > Additionally I added a test that uses (on Windows) JLI_Open on a
> > > jar
> > file
> > > with a "long" path (> 260 chars).
> > > [ JLI_Open is currently called from parse_manifest.c with jarfile as
> > > parameter ]
> > >
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Matthias,
the change to src/java.base/windows/native/libjli/java_md.c looks good to me
now.
As for the test, I think you could also check a jar in a short path to exercise
both cases in JLI_Open.
And a few nits:
Line 506: better do:
Path pelp = pcreated.resolve(elp.jar);
Line 507:
2 spaces are one too much after elp
Line 508:
Insert 2 additional spaces in the source code string, a space between "){" and
another in the end after the ; of the System.out.println statement, like this:
createJar(elp, new File("Foo"), "public static void main(String[] args) {
System.out.println(\"Hello from ELP\"); }");
You could also do a little cleanup: Remove method private static String
removeExtraQuotes(String in) { from line 64. It seems, it is not needed.
Best regards
Christoph
> -Original Message-
> From: Baesken, Matthias
> Sent: Mittwoch, 27. März 2019 18:05
> To: Langer, Christoph
> Cc: core-libs-dev@openjdk.java.net; Alan Bateman
>
> Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> Hello could you please look into it ?
>
> Best regards, Matthias
>
>
>
> > -Original Message-
> > From: Baesken, Matthias
> > Sent: Montag, 25. März 2019 18:05
> > To: Langer, Christoph
> > Cc: 'core-libs-dev@openjdk.java.net' ;
> > 'Alan Bateman'
> > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code
> > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > Hello here is an updated webrev :
> >
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.1/webrev/
> >
> >
> >
> > > > In JLI_Open(const char* name, int flags), you should remove ret and
> only
> > > > use fd, I think.
> >
> >
> > I removed ret and adjusted some comments.
> >
> > Additionally I added a test that uses (on Windows) JLI_Open on a
> > jar
> file
> > with a "long" path (> 260 chars).
> > [ JLI_Open is currently called from parse_manifest.c with jarfile as
> > parameter ]
> >
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello could you please look into it ? Best regards, Matthias > -Original Message- > From: Baesken, Matthias > Sent: Montag, 25. März 2019 18:05 > To: Langer, Christoph > Cc: 'core-libs-dev@openjdk.java.net' ; > 'Alan Bateman' > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > Hello here is an updated webrev : > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.1/webrev/ > > > > > > In JLI_Open(const char* name, int flags), you should remove ret and only > > > use fd, I think. > > > I removed ret and adjusted some comments. > > Additionally I added a test that uses (on Windows) JLI_Open on a jar > file > with a "long" path (> 260 chars). > [ JLI_Open is currently called from parse_manifest.c with jarfile as > parameter ] >
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello here is an updated webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.1/webrev/ > > In JLI_Open(const char* name, int flags), you should remove ret and only > > use fd, I think. I removed ret and adjusted some comments. Additionally I added a test that uses (on Windows) JLI_Open on a jar file with a "long" path (> 260 chars). [ JLI_Open is currently called from parse_manifest.c with jarfile as parameter ] Best regards, Matthias > -Original Message- > From: Baesken, Matthias > Sent: Montag, 25. März 2019 10:48 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net; Alan Bateman > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > Hi Christoph / Alan, thanks for the reviews. > > > Btw what do you think about the longPathAware setting in the manifest > ? > > https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file > > https://blogs.msdn.microsoft.com/jeremykuhne/2016/07/30/net-4-6-2-and- > long-paths-on-windows-10/ > > This might be an option for Windows 10 / Windows server 2016 to enable > long path awareness for all open calls. > ( however for older Windows versions I think we still need the ELP/UNC path > coding) > > > Thanks, Matthias > > > > > -Original Message- > > From: Langer, Christoph > > Sent: Sonntag, 24. März 2019 07:14 > > To: Baesken, Matthias > > Cc: core-libs-dev@openjdk.java.net; Alan Bateman > > > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in > > parse_manifest.c on windows > > > > Hi Matthias, > > > > yes, I think this is generally a good way to go. > > > > In JLI_Open(const char* name, int flags), you should remove ret and only > > use fd, I think. (Currently it would return 0 in the _wopen case). > > > > Furthermore, I think it would be a good time to introduce a test now that > > exercises paths to libjvm of different lengths... > > > > Thanks for your work here! > > > > Best regards > > Christoph > > > > > > > -Original Message----- > > > From: Alan Bateman > > > Sent: Freitag, 22. März 2019 20:37 > > > To: Baesken, Matthias ; Langer, Christoph > > > > > > Cc: core-libs-dev@openjdk.java.net > > > Subject: Re: RFR: 8218547: Simplify JLI_Open on Windows in native code > > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in > > > parse_manifest.c on windows > > > > > > On 22/03/2019 14:37, Baesken, Matthias wrote: > > > > Hello, here is the new webrev . > > > > > > > > I took over and adjusted coding from os::open + > > > > create_unc_path > > > functions in os_windows.cpp in hotspot : > > > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.0/webrev/ > > > > > > > This looks quite good. For the comment then it might be better to drop > > > the mention of os_windows.cpp, also maybe mention that the memory > > > should > > > be freed with JLI_MemFree rather than free. > > > > > > -Alan
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Christoph / Alan, thanks for the reviews. Btw what do you think about the longPathAware setting in the manifest ? https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file https://blogs.msdn.microsoft.com/jeremykuhne/2016/07/30/net-4-6-2-and-long-paths-on-windows-10/ This might be an option for Windows 10 / Windows server 2016 to enable long path awareness for all open calls. ( however for older Windows versions I think we still need the ELP/UNC path coding) Thanks, Matthias > -Original Message- > From: Langer, Christoph > Sent: Sonntag, 24. März 2019 07:14 > To: Baesken, Matthias > Cc: core-libs-dev@openjdk.java.net; Alan Bateman > > Subject: RE: RFR: 8218547: Simplify JLI_Open on Windows in native code > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > Hi Matthias, > > yes, I think this is generally a good way to go. > > In JLI_Open(const char* name, int flags), you should remove ret and only > use fd, I think. (Currently it would return 0 in the _wopen case). > > Furthermore, I think it would be a good time to introduce a test now that > exercises paths to libjvm of different lengths... > > Thanks for your work here! > > Best regards > Christoph > > > > -Original Message- > > From: Alan Bateman > > Sent: Freitag, 22. März 2019 20:37 > > To: Baesken, Matthias ; Langer, Christoph > > > > Cc: core-libs-dev@openjdk.java.net > > Subject: Re: RFR: 8218547: Simplify JLI_Open on Windows in native code > > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in > > parse_manifest.c on windows > > > > On 22/03/2019 14:37, Baesken, Matthias wrote: > > > Hello, here is the new webrev . > > > > > > I took over and adjusted coding from os::open + create_unc_path > > functions in os_windows.cpp in hotspot : > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.0/webrev/ > > > > > This looks quite good. For the comment then it might be better to drop > > the mention of os_windows.cpp, also maybe mention that the memory > > should > > be freed with JLI_MemFree rather than free. > > > > -Alan
RE: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Matthias, yes, I think this is generally a good way to go. In JLI_Open(const char* name, int flags), you should remove ret and only use fd, I think. (Currently it would return 0 in the _wopen case). Furthermore, I think it would be a good time to introduce a test now that exercises paths to libjvm of different lengths... Thanks for your work here! Best regards Christoph > -Original Message- > From: Alan Bateman > Sent: Freitag, 22. März 2019 20:37 > To: Baesken, Matthias ; Langer, Christoph > > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8218547: Simplify JLI_Open on Windows in native code > (libjli) - was : RE: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > On 22/03/2019 14:37, Baesken, Matthias wrote: > > Hello, here is the new webrev . > > > > I took over and adjusted coding from os::open + create_unc_path > functions in os_windows.cpp in hotspot : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.0/webrev/ > > > This looks quite good. For the comment then it might be better to drop > the mention of os_windows.cpp, also maybe mention that the memory > should > be freed with JLI_MemFree rather than free. > > -Alan
Re: RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 22/03/2019 14:37, Baesken, Matthias wrote: Hello, here is the new webrev . I took over and adjusted coding from os::open + create_unc_path functions in os_windows.cpp in hotspot : http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.0/webrev/ This looks quite good. For the comment then it might be better to drop the mention of os_windows.cpp, also maybe mention that the memory should be freed with JLI_MemFree rather than free. -Alan
RFR: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello, here is the new webrev .
I took over and adjusted coding from os::open + create_unc_path
functions in os_windows.cpp in hotspot :
http://cr.openjdk.java.net/~mbaesken/webrevs/8218547.0/webrev/
Best regards, Matthias
> -Original Message-
> From: Baesken, Matthias
> Sent: Donnerstag, 21. März 2019 14:30
> To: Langer, Christoph ; Alan Bateman
> (alan.bate...@oracle.com)
> Cc: 'Chris Hegarty' ; 'core-libs-
> d...@openjdk.java.net'
> Subject: 8218547: Simplify JLI_Open on Windows in native code (libjli) - was :
> RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on
> windows
>
> Hi Alan + Christoph, coming back to
> https://bugs.openjdk.java.net/browse/JDK-8218547 .
>
> We wanted to check for the whole flow (in JLI_Open on Windows ) , e.g.
> checking whether only CreateFileW can be used instead of open.
>
> I think there is already a similar solution available in the codebase .
>
> See the os::open + create_unc_path functions in os_windows.cpp in
> the hotspot coding .
>
> There "simple" open is used for shorter paths ( < MAX_PATH) ; for
> larger
> paths an elp / unc path is constructed and _wopen is used on the elp/unc
> path :
>
>
> if (strlen(path) < MAX_PATH) {
> ret = ::open(pathbuf, oflag | O_BINARY | O_NOINHERIT, mode);
> } else {
> errno_t err = ERROR_SUCCESS;
> wchar_t* wpath = create_unc_path(pathbuf, err);
>
> ret = ::_wopen(wpath, oflag | O_BINARY | O_NOINHERIT, mode);
>
>
> I think we should do it the same way in JLI_Open . Is that fine with you?
> Unfortunately I think we cannot easily reuse the HS coding, so the have
> to
> take it over to jli .
>
> Best regards, Matthias
>
>
>
> > -Original Message-
> > From: Baesken, Matthias
> > Sent: Mittwoch, 6. Februar 2019 09:56
> > To: Langer, Christoph
> > Cc: Chris Hegarty ; core-libs-
> > d...@openjdk.java.net; Alan Bateman (alan.bate...@oracle.com)
> >
> > Subject: RE: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > Hi Christoph+Alan, I opened
> >
> >
> > https://bugs.openjdk.java.net/browse/JDK-8218547
> >
> > JDK-8218547 : use CreateFile without open on Windows in jdk native code
> >
> >
> > To check for the usage of CreateFile (CreateFileW) without open .
> >
> >
> > Best regards, Matthias
> >
8218547: Simplify JLI_Open on Windows in native code (libjli) - was : RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Alan + Christoph, coming back to
https://bugs.openjdk.java.net/browse/JDK-8218547 .
We wanted to check for the whole flow (in JLI_Open on Windows ) , e.g.
checking whether only CreateFileW can be used instead of open.
I think there is already a similar solution available in the codebase .
See the os::open + create_unc_path functions in os_windows.cpp in
the hotspot coding .
There "simple" open is used for shorter paths ( < MAX_PATH) ; for
larger paths an elp / unc path is constructed and _wopen is used on the
elp/unc path :
if (strlen(path) < MAX_PATH) {
ret = ::open(pathbuf, oflag | O_BINARY | O_NOINHERIT, mode);
} else {
errno_t err = ERROR_SUCCESS;
wchar_t* wpath = create_unc_path(pathbuf, err);
ret = ::_wopen(wpath, oflag | O_BINARY | O_NOINHERIT, mode);
I think we should do it the same way in JLI_Open . Is that fine with you?
Unfortunately I think we cannot easily reuse the HS coding, so the have to
take it over to jli .
Best regards, Matthias
> -Original Message-
> From: Baesken, Matthias
> Sent: Mittwoch, 6. Februar 2019 09:56
> To: Langer, Christoph
> Cc: Chris Hegarty ; core-libs-
> d...@openjdk.java.net; Alan Bateman (alan.bate...@oracle.com)
>
> Subject: RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> Hi Christoph+Alan, I opened
>
>
> https://bugs.openjdk.java.net/browse/JDK-8218547
>
> JDK-8218547 : use CreateFile without open on Windows in jdk native code
>
>
> To check for the usage of CreateFile (CreateFileW) without open .
>
>
> Best regards, Matthias
>
>
> > -Original Message-
> > From: Langer, Christoph
> > Sent: Dienstag, 29. Januar 2019 09:59
> > To: Baesken, Matthias
> > Cc: Chris Hegarty ; core-libs-
> > d...@openjdk.java.net
> > Subject: RE: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > Hi Matthias,
> >
> > > > New webrev :
> > > >
> > > >
> > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/
> > >
> > > This is the one. Looks good ( and clean ).
> >
> > I agree, this version would be ok for me to be pushed. It would be good
> > ,though, if there was a test for this (long paths on Windows).
> >
> > I also like Alan's suggestion to open a follow up bug to explore using
> > CreateFile on Windows right away rather than trying open. Looking at
> > libjava/libnet/libnio, it's all CreateFileW on Windows...
> >
> > Best regards
> > Christoph
> >
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Thanks .
Looks like there are a few open calls in windows or shared code present , for
example :
Zlib:
gzlib.c
hotspot:
osSupport_windows.cpp
compactHashtable.cpp
but mostly it is CreateFile(W) ( not sure if it is always with extended length
path support).
We could also compare what is done in JDK and in Windows HS os::open
int os::open(const char *path, int oflag, int mode) {
from os_windows.cpp(this creates an UNC path + uses _wopen ).
Best regards, Matthias
> -Original Message-
> From: Langer, Christoph
> Sent: Mittwoch, 6. Februar 2019 10:08
> To: Baesken, Matthias
> Cc: core-libs-dev@openjdk.java.net; Alan Bateman
>
> Subject: RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> Hi Matthias,
>
> thanks for opening the bug. I refined its description a little...
>
> Best regards
> Christoph
>
> > -Original Message-
> > From: Alan Bateman
> > Sent: Mittwoch, 6. Februar 2019 10:00
> > To: Langer, Christoph ; Baesken, Matthias
> >
> > Cc: core-libs-dev@openjdk.java.net
> > Subject: Re: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> >
> > On 29/01/2019 08:58, Langer, Christoph wrote:
> > > Hi Matthias,
> > >
> > >>> New webrev :
> > >>>
> > >>>
> > >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/
> > >> This is the one. Looks good ( and clean ).
> > > I agree, this version would be ok for me to be pushed. It would be good
> > ,though, if there was a test for this (long paths on Windows).
> > >
> > > I also like Alan's suggestion to open a follow up bug to explore using
> > CreateFile on Windows right away rather than trying open. Looking at
> > libjava/libnet/libnio, it's all CreateFileW on Windows...
> > I think the right fix is to convert a relative path into absolute path,
> > apply the prefix for long paths, and then use CreateFileW consistently.
> > It's probably okay to get there in steps but I can't tell from the
> > current webrev if it works with relative paths or not.
> >
> > -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Matthias, thanks for opening the bug. I refined its description a little... Best regards Christoph > -Original Message- > From: Alan Bateman > Sent: Mittwoch, 6. Februar 2019 10:00 > To: Langer, Christoph ; Baesken, Matthias > > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > > On 29/01/2019 08:58, Langer, Christoph wrote: > > Hi Matthias, > > > >>> New webrev : > >>> > >>> > >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/ > >> This is the one. Looks good ( and clean ). > > I agree, this version would be ok for me to be pushed. It would be good > ,though, if there was a test for this (long paths on Windows). > > > > I also like Alan's suggestion to open a follow up bug to explore using > CreateFile on Windows right away rather than trying open. Looking at > libjava/libnet/libnio, it's all CreateFileW on Windows... > I think the right fix is to convert a relative path into absolute path, > apply the prefix for long paths, and then use CreateFileW consistently. > It's probably okay to get there in steps but I can't tell from the > current webrev if it works with relative paths or not. > > -Alan
Re: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 29/01/2019 08:58, Langer, Christoph wrote: Hi Matthias, New webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/ This is the one. Looks good ( and clean ). I agree, this version would be ok for me to be pushed. It would be good ,though, if there was a test for this (long paths on Windows). I also like Alan's suggestion to open a follow up bug to explore using CreateFile on Windows right away rather than trying open. Looking at libjava/libnet/libnio, it's all CreateFileW on Windows... I think the right fix is to convert a relative path into absolute path, apply the prefix for long paths, and then use CreateFileW consistently. It's probably okay to get there in steps but I can't tell from the current webrev if it works with relative paths or not. -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Christoph+Alan, I opened https://bugs.openjdk.java.net/browse/JDK-8218547 JDK-8218547 : use CreateFile without open on Windows in jdk native code To check for the usage of CreateFile (CreateFileW) without open . Best regards, Matthias > -Original Message- > From: Langer, Christoph > Sent: Dienstag, 29. Januar 2019 09:59 > To: Baesken, Matthias > Cc: Chris Hegarty ; core-libs- > d...@openjdk.java.net > Subject: RE: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > Hi Matthias, > > > > New webrev : > > > > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/ > > > > This is the one. Looks good ( and clean ). > > I agree, this version would be ok for me to be pushed. It would be good > ,though, if there was a test for this (long paths on Windows). > > I also like Alan's suggestion to open a follow up bug to explore using > CreateFile on Windows right away rather than trying open. Looking at > libjava/libnet/libnio, it's all CreateFileW on Windows... > > Best regards > Christoph >
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Matthias, > > New webrev : > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/ > > This is the one. Looks good ( and clean ). I agree, this version would be ok for me to be pushed. It would be good ,though, if there was a test for this (long paths on Windows). I also like Alan's suggestion to open a follow up bug to explore using CreateFile on Windows right away rather than trying open. Looking at libjava/libnet/libnio, it's all CreateFileW on Windows... Best regards Christoph
Re: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 28/01/2019 16:09, Baesken, Matthias wrote: you should think of a JLI_open function and then you can put its implementation into the Windows version of java_md.c. Hi Christoph, I like this idea . New webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/ This is the one. Looks good ( and clean ). -Chris.
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
> you should think of a JLI_open function and then you can > put its implementation into the Windows version of java_md.c. Hi Christoph, I like this idea . New webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.4/ > > Would it be possible to add a jtreg testcase that shows/tests the issue? > Maybe I could put something into the existing jdk/test/jdk/tools/launcher tests. Best regards, Matthias > -Original Message- > From: Langer, Christoph > Sent: Montag, 28. Januar 2019 14:23 > To: Baesken, Matthias ; Alan Bateman > ; core-libs-dev@openjdk.java.net > Subject: RE: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > Hi Matthias, > > thanks for the update. > > I still think you should avoid introducing a file parse_manifest_md.c. As per > my previous mail, you should think of a JLI_open function and then you can > put its implementation into the Windows version of java_md.c. You can take > example on the implementation of JLI_Snprintf which has coding on > Windows and for Unix it's just a #define to snprintf. > > Would it be possible to add a jtreg testcase that shows/tests the issue? > > Best regards > Christoph > > > -Original Message- > > From: Baesken, Matthias > > Sent: Montag, 28. Januar 2019 12:38 > > To: Alan Bateman ; core-libs- > > d...@openjdk.java.net > > Cc: Langer, Christoph > > Subject: RE: RFR : 8217093: Support extended-length paths in > > parse_manifest.c on windows > > > > Hello Alan / Christoph, > > new webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.3/ > > > > - moved ELP_PREFIX define out of the function > > - added a check(wfullnamelen > 247) before CreateFileW is attempted > > - some other formatting (mostly suggested by Christoph) > > > > > > Best regards, Matthias > > > > > > > -Original Message----- > > > From: Alan Bateman > > > Sent: Sonntag, 27. Januar 2019 16:57 > > > To: Baesken, Matthias ; core-libs- > > > d...@openjdk.java.net > > > Cc: Langer, Christoph > > > Subject: Re: RFR : 8217093: Support extended-length paths in > > > parse_manifest.c on windows > > > > > > On 24/01/2019 16:19, Baesken, Matthias wrote: > > > >> Did you consider adding a Unix version of open_jarfile to avoid #ifdef > > > >> WIN32 ? > > > > Hi Alan, do you mean , adding a separate c-file for UNIX containing the > > > function open_jarfile ? > > > > I considered this, but did not like the idea to add a separate file for > UNIX > > > just for the small function . > > > > > > > > Your suggestion to use FILE_SHARE_READ is probably the right way to > > do > > > it , I changed this, second webrev : > > > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.2/ > > > > > > > Christoph Langer's suggestion to use jli_util seems worth exploring to > > > keep the platform specific code together. > > > > > > I still find the call to _open and checking for ENOENT a bit icky. If > > > this change gets pushed then I think we need a follow-up issue created > > > to replace this to use CreateFile consistently. > > > > > > -Alan >
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Matthias, thanks for the update. I still think you should avoid introducing a file parse_manifest_md.c. As per my previous mail, you should think of a JLI_open function and then you can put its implementation into the Windows version of java_md.c. You can take example on the implementation of JLI_Snprintf which has coding on Windows and for Unix it's just a #define to snprintf. Would it be possible to add a jtreg testcase that shows/tests the issue? Best regards Christoph > -Original Message- > From: Baesken, Matthias > Sent: Montag, 28. Januar 2019 12:38 > To: Alan Bateman ; core-libs- > d...@openjdk.java.net > Cc: Langer, Christoph > Subject: RE: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > Hello Alan / Christoph, > new webrev : > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.3/ > > - moved ELP_PREFIX define out of the function > - added a check(wfullnamelen > 247) before CreateFileW is attempted > - some other formatting (mostly suggested by Christoph) > > > Best regards, Matthias > > > > -Original Message- > > From: Alan Bateman > > Sent: Sonntag, 27. Januar 2019 16:57 > > To: Baesken, Matthias ; core-libs- > > d...@openjdk.java.net > > Cc: Langer, Christoph > > Subject: Re: RFR : 8217093: Support extended-length paths in > > parse_manifest.c on windows > > > > On 24/01/2019 16:19, Baesken, Matthias wrote: > > >> Did you consider adding a Unix version of open_jarfile to avoid #ifdef > > >> WIN32 ? > > > Hi Alan, do you mean , adding a separate c-file for UNIX containing the > > function open_jarfile ? > > > I considered this, but did not like the idea to add a separate file for > > > UNIX > > just for the small function . > > > > > > Your suggestion to use FILE_SHARE_READ is probably the right way to > do > > it , I changed this, second webrev : > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.2/ > > > > > Christoph Langer's suggestion to use jli_util seems worth exploring to > > keep the platform specific code together. > > > > I still find the call to _open and checking for ENOENT a bit icky. If > > this change gets pushed then I think we need a follow-up issue created > > to replace this to use CreateFile consistently. > > > > -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello Alan / Christoph, new webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.3/ - moved ELP_PREFIX define out of the function - added a check(wfullnamelen > 247) before CreateFileW is attempted - some other formatting (mostly suggested by Christoph) Best regards, Matthias > -Original Message- > From: Alan Bateman > Sent: Sonntag, 27. Januar 2019 16:57 > To: Baesken, Matthias ; core-libs- > d...@openjdk.java.net > Cc: Langer, Christoph > Subject: Re: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > On 24/01/2019 16:19, Baesken, Matthias wrote: > >> Did you consider adding a Unix version of open_jarfile to avoid #ifdef > >> WIN32 ? > > Hi Alan, do you mean , adding a separate c-file for UNIX containing the > function open_jarfile ? > > I considered this, but did not like the idea to add a separate file for UNIX > just for the small function . > > > > Your suggestion to use FILE_SHARE_READ is probably the right way to do > it , I changed this, second webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.2/ > > > Christoph Langer's suggestion to use jli_util seems worth exploring to > keep the platform specific code together. > > I still find the call to _open and checking for ENOENT a bit icky. If > this change gets pushed then I think we need a follow-up issue created > to replace this to use CreateFile consistently. > > -Alan
Re: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 24/01/2019 16:19, Baesken, Matthias wrote: Did you consider adding a Unix version of open_jarfile to avoid #ifdef WIN32 ? Hi Alan, do you mean , adding a separate c-file for UNIX containing the function open_jarfile ? I considered this, but did not like the idea to add a separate file for UNIX just for the small function . Your suggestion to use FILE_SHARE_READ is probably the right way to do it , I changed this, second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.2/ Christoph Langer's suggestion to use jli_util seems worth exploring to keep the platform specific code together. I still find the call to _open and checking for ENOENT a bit icky. If this change gets pushed then I think we need a follow-up issue created to replace this to use CreateFile consistently. -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Matthias,
thanks for bringing this enhancement into the OpenJDK. I have a few wishes,
though.
First of all, I think this method fits perfectly well to jli_util. It does not
really have to do with the logic of parsing manifests but it is rather an
enhanced open function.
So you should declare a function "int JLI_Open (const char* name, int flags);"
in jli_util.h. Then, create a platform file jli_util_md.c, at least for Windows
(instead of parse_manifest_md.c). I also rather like the idea to symmetrically
create a jli_util_md.c file for Unix, although it has very little contents.
Alternatively, you can maybe consider using "#define JLI_Open open" in the
header file for the Unix version.
As for the Windows implementation I have a few stylistic comments:
40 if (fd == -1 && ENOENT == errno) {
I'd rather like
40 if (fd == -1 && errno == ENOENT) {
41 #define ELP_PREFIX L"?\\"
Can you move this line out of the function, after the includes?
Can you change the indentation of:
64 wfullname_with_prefix = (wchar_t*) malloc((wcslen(ELP_PREFIX) +
wfullnamelen + 1)
65 * sizeof(wchar_t));
to:
64 wfullname_with_prefix = (wchar_t*) malloc(
65 (wcslen(ELP_PREFIX) + wfullnamelen + 1) * sizeof(wchar_t));
I also don't like the label/goto syntax so much... don't know if you would want
to rewrite this a bit? Maybe you can then also pick up Alan's suggestion to
check for the length of 247 and only add the prefix in case it's equal or
larger which would save you a malloc operation.
Thanks
Christoph
> -Original Message-
> From: Baesken, Matthias
> Sent: Donnerstag, 24. Januar 2019 17:19
> To: Alan Bateman ; core-libs-
> d...@openjdk.java.net
> Cc: Langer, Christoph
> Subject: RE: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
>
> > Did you consider adding a Unix version of open_jarfile to avoid #ifdef
> > WIN32 ?
>
> Hi Alan, do you mean , adding a separate c-file for UNIX containing the
> function open_jarfile ?
> I considered this, but did not like the idea to add a separate file for UNIX
> just
> for the small function .
>
> Your suggestion to use FILE_SHARE_READ is probably the right way to do it ,
> I changed this, second webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.2/
>
>
> Best Regards, Matthias
>
>
> > -Original Message-
> > From: Alan Bateman
> > Sent: Donnerstag, 24. Januar 2019 13:04
> > To: Baesken, Matthias ; core-libs-
> > d...@openjdk.java.net
> > Subject: Re: RFR : 8217093: Support extended-length paths in
> > parse_manifest.c on windows
> >
> > On 23/01/2019 08:29, Baesken, Matthias wrote:
> > > Hello Alan, here is a second webrev :
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.1/
> > >
> > > I moved the Windows-only code into a new file
> > >
> > > src/java.base/windows/native/libjli/parse_manifest_md.c
> > >
> > Did you consider adding a Unix version of open_jarfile to avoid #ifdef
> > WIN32 ?
> >
> > On the Windows version then your patch is low risk but the retrying
> > after ENOENT is a bit icky. I'm just wondering if you've considered
> > creating an absolute path and adding the prefix when the length is 247
> > or greater. I also wonder about the share mode specified to CreateFileW
> > where it might be better to specify FILE_SHARE_READ at least so that you
> > don't exclude other processes from also opening the file to read.
> >
> > -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
> Did you consider adding a Unix version of open_jarfile to avoid #ifdef > WIN32 ? Hi Alan, do you mean , adding a separate c-file for UNIX containing the function open_jarfile ? I considered this, but did not like the idea to add a separate file for UNIX just for the small function . Your suggestion to use FILE_SHARE_READ is probably the right way to do it , I changed this, second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.2/ Best Regards, Matthias > -Original Message- > From: Alan Bateman > Sent: Donnerstag, 24. Januar 2019 13:04 > To: Baesken, Matthias ; core-libs- > d...@openjdk.java.net > Subject: Re: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > On 23/01/2019 08:29, Baesken, Matthias wrote: > > Hello Alan, here is a second webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.1/ > > > > I moved the Windows-only code into a new file > > > > src/java.base/windows/native/libjli/parse_manifest_md.c > > > Did you consider adding a Unix version of open_jarfile to avoid #ifdef > WIN32 ? > > On the Windows version then your patch is low risk but the retrying > after ENOENT is a bit icky. I'm just wondering if you've considered > creating an absolute path and adding the prefix when the length is 247 > or greater. I also wonder about the share mode specified to CreateFileW > where it might be better to specify FILE_SHARE_READ at least so that you > don't exclude other processes from also opening the file to read. > > -Alan
Re: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 23/01/2019 08:29, Baesken, Matthias wrote: Hello Alan, here is a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.1/ I moved the Windows-only code into a new file src/java.base/windows/native/libjli/parse_manifest_md.c Did you consider adding a Unix version of open_jarfile to avoid #ifdef WIN32 ? On the Windows version then your patch is low risk but the retrying after ENOENT is a bit icky. I'm just wondering if you've considered creating an absolute path and adding the prefix when the length is 247 or greater. I also wonder about the share mode specified to CreateFileW where it might be better to specify FILE_SHARE_READ at least so that you don't exclude other processes from also opening the file to read. -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello Alan, here is a second webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.1/ I moved the Windows-only code into a new file src/java.base/windows/native/libjli/parse_manifest_md.c Best regards, Matthias > -Original Message- > From: Alan Bateman > Sent: Mittwoch, 16. Januar 2019 16:43 > To: Baesken, Matthias ; core-libs- > d...@openjdk.java.net > Subject: Re: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > On 16/01/2019 14:43, Baesken, Matthias wrote: > > Hi Alan , do you think it is good to start a UNIX parse_manifest_md.c too > with just a one-liner in it ? > > Or do you prefer only having the windows - file with the Windows coding > and keeping the UNIX one-linerfor open_jarfile in parse_manifest.c > ? > > > I think this makes sense. > > -Alan
Re: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 16/01/2019 14:43, Baesken, Matthias wrote: Hi Alan , do you think it is good to start a UNIX parse_manifest_md.c too with just a one-liner in it ? Or do you prefer only having the windows - file with the Windows coding and keeping the UNIX one-linerfor open_jarfile in parse_manifest.c ? I think this makes sense. -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Alan , do you think it is good to start a UNIX parse_manifest_md.c too with just a one-liner in it ? Or do you prefer only having the windows - file with the Windows coding and keeping the UNIX one-linerfor open_jarfile in parse_manifest.c ? Best regards, Matthias > -Original Message- > From: Alan Bateman > Sent: Mittwoch, 16. Januar 2019 10:37 > To: Baesken, Matthias ; core-libs- > d...@openjdk.java.net > Subject: Re: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > On 16/01/2019 09:07, Baesken, Matthias wrote: > > Hi Alan, I thought having UNIX+Windows code in this case together at > one place would be okay . > > However, if you prefer separating it, should I put it here ? > > > > src/java.base/windows/native/libjli/java_md.c > > > > Or do you have a better suggestion ? > > > With a few exceptions, the Windows specific code for jli.dll is in > src/java.base/windows/native/libji so if it's not too difficult then I > think that should be location for the Windows long path support. It's > might be parse_manifest_md.c rather than java_md.c as the latter is the > platform specific code to go with launcher code in java.c. > > -Alan
Re: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 16/01/2019 09:07, Baesken, Matthias wrote: Hi Alan, I thought having UNIX+Windows code in this case together at one place would be okay . However, if you prefer separating it, should I put it here ? src/java.base/windows/native/libjli/java_md.c Or do you have a better suggestion ? With a few exceptions, the Windows specific code for jli.dll is in src/java.base/windows/native/libji so if it's not too difficult then I think that should be location for the Windows long path support. It's might be parse_manifest_md.c rather than java_md.c as the latter is the platform specific code to go with launcher code in java.c. -Alan
RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hi Alan, I thought having UNIX+Windows code in this case together at one place would be okay . However, if you prefer separating it, should I put it here ? src/java.base/windows/native/libjli/java_md.c Or do you have a better suggestion ? Best regards, Matthias > From: Alan Bateman > Sent: Mittwoch, 16. Januar 2019 09:51 > To: Baesken, Matthias ; core-libs- > d...@openjdk.java.net > Subject: Re: RFR : 8217093: Support extended-length paths in > parse_manifest.c on windows > > On 16/01/2019 08:42, Baesken, Matthias wrote: > > Hello, please review this small adjustment to parse_manifest.c . > > It adds support for extended - length paths (on Window) . > > > > (see https://docs.microsoft.com/en-us/windows/desktop/fileio/naming- > a-file#maximum-path-length-limitation > > For extended length paths ) > > > > Additionally , some small memory leaks are fixed in find_file . > > > > Bug/webrev : > > > > https://bugs.openjdk.java.net/browse/JDK-8217093 > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.0/ > > > If I read this correctly then it will prepend \\?\ which is consistent > with what we do in other areas of the platform when presented with a > long file name. Have you looked at moving open_jarfile to a platform > specific source file?, in this case it might be better if the WIndows > specific code were in src/java.base/windows/native/libjli for example. > > -Alan
Re: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
On 16/01/2019 08:42, Baesken, Matthias wrote: Hello, please review this small adjustment to parse_manifest.c . It adds support for extended - length paths (on Window) . (see https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file#maximum-path-length-limitation For extended length paths ) Additionally , some small memory leaks are fixed in find_file . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8217093 http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.0/ If I read this correctly then it will prepend \\?\ which is consistent with what we do in other areas of the platform when presented with a long file name. Have you looked at moving open_jarfile to a platform specific source file?, in this case it might be better if the WIndows specific code were in src/java.base/windows/native/libjli for example. -Alan
RFR : 8217093: Support extended-length paths in parse_manifest.c on windows
Hello, please review this small adjustment to parse_manifest.c . It adds support for extended - length paths (on Window) . (see https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file#maximum-path-length-limitation For extended length paths ) Additionally , some small memory leaks are fixed in find_file . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8217093 http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.0/ Best rergards, Matthias
