Thank you Volker :) On Wed, Feb 11, 2015 at 2:36 PM, Volker Simonis <volker.simo...@gmail.com> wrote:
> 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 > > > > >
