Hi Thomas, I've just notices that your test has no copyright info and is indented with an indent width of 2 spaces instead of 4. If you don't mind I'll fix this before pushing so there's no need for a new webrev.
Regards, Volker On Tue, Feb 10, 2015 at 10:58 AM, Thomas Stüfe <thomas.stu...@gmail.com> wrote: > > Hi Roger, Volker, > > thanks for reviewing! > > I added all requested changes: > > @Roger > - use now Files.createTempDirectory to get a unique directory > - wrapped test in try/finally to cleanup afterwards > @Volker > - moved include up and added dependency to comment in io_util_md.h > - Now I use hostname.exe, which I hope is part of every Windows :) > > http://cr.openjdk.java.net/~stuefe/webrevs/8072611/webrev.02/webrev/ > > Regards, Thomas > > > On Mon, Feb 9, 2015 at 7:48 PM, Volker Simonis <volker.simo...@gmail.com> > wrote: >> >> Hi Thomas, >> >> the change looks good and I can sponsor it once it is reviewed. >> >> Just some small notes: >> >> - it seems that "getPath()" isn't used anywhere else in >> ProcessImpl_md.c and because it is static it can't be used anywhere >> else. So can you please remove it completely. >> >> - io_util_md.h has a list of files which use the prototypes like >> "pathToNTPath" before the declaration of the prototypes. Could you >> please also add ProcessImpl_md.c to this list >> >> - can you pleae place the new include in ProcessImpl_md.c as follows: >> >> #include "io_util.h" >> +#include "io_util_md.h" >> #include <windows.h> >> #include <io.h> >> >> I saw that system and local headers are already intermixed in that >> file but at I think at least we shouldn't introduce more disorder. >> >> - is "robocopy" really available on all supported Windows system. In >> http://en.wikipedia.org/wiki/Robocopy I read that it was introduced in >> Server 2008. I just verified that Java 8 only supports Server 2008 and >> above but just think of our internal test system:) Maybe we can use a >> program which is supported in every Windows version? >> >> Regards, >> Volker >> >> >> On Mon, Feb 9, 2015 at 2:30 PM, Thomas Stüfe <thomas.stu...@gmail.com> >> wrote: >> > Hi all, >> > >> > please review this small change at your convenience: >> > >> > http://cr.openjdk.java.net/~stuefe/webrevs/8072611/webrev.01/webrev/ >> > >> > It fixes a small issue which causes ProcessBuilder not to be able to >> > open >> > files to redirect into (when using Redirect.append) on windows, if that >> > file name is longer than 255 chars. >> > >> > Kind Regards, Thomas Stuefe > >
