On 1 December 2011 08:59, Philippe Mouawad <[email protected]> wrote: > Hello Sebb, > I checked code before doing this and method is called from > <initializeFileOutput (private) < testStarted (public) in synchronized > block so it is OK like this as no other method calls it.
No, it's not OK. - synch. does not help here. Some external process can change the directory structure. Highly unlikely, but not impossible. > But if you think it should be as it was, I can rollback. Yes, please do. > Regards > Philippe > > On Thu, Dec 1, 2011 at 1:01 AM, sebb <[email protected]> wrote: > >> On 30 November 2011 22:16, <[email protected]> wrote: >> > Author: pmouawad >> > Date: Wed Nov 30 22:16:35 2011 >> > New Revision: 1208836 >> > >> > URL: http://svn.apache.org/viewvc?rev=1208836&view=rev >> > Log: >> > Made code cleaner >> >> Perhaps, but it introduces a subtle bug, see below. >> >> > Modified: >> > jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java >> > >> > Modified: >> jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java?rev=1208836&r1=1208835&r2=1208836&view=diff >> > >> ============================================================================== >> > --- >> jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java >> (original) >> > +++ >> jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java Wed >> Nov 30 22:16:35 2011 >> > @@ -389,10 +389,10 @@ public class ResultCollector extends Abs >> > // Find the name of the directory containing the file >> > // and create it - if there is one >> > File pdir = new File(filename).getParentFile(); >> > - if (pdir != null) { >> > - pdir.mkdirs();// returns false if directory already >> exists, so need to check again >> > - if (!pdir.exists()){ >> > - log.warn("Error creating directories for >> "+pdir.toString()); >> > + if (pdir != null && !pdir.exists()) { >> > + boolean mkdirResult = pdir.mkdirs(); >> > + if (!mkdirResult){ >> >> It's not totally safe to check the return from mkdirs() after checking >> exists(). >> See for example: >> >> https://issues.apache.org/jira/browse/JCI-67 >> >> Please revert the change. >> >> > + log.warn("Error creating directories for >> "+pdir.getAbsolutePath()); >> > } >> > } >> > writer = new PrintWriter(new OutputStreamWriter(new >> BufferedOutputStream(new FileOutputStream(filename, >> > >> > >> > > > > -- > Cordialement. > Philippe Mouawad.
