jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1208836 - /jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java
Date Thu, 01 Dec 2011 08:59:26 GMT
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.

But if you think it should be as it was, I can rollback.

Regards
Philippe

On Thu, Dec 1, 2011 at 1:01 AM, sebb <sebbaz@gmail.com> wrote:

> On 30 November 2011 22:16,  <pmouawad@apache.org> 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.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message