jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1520921 - /jmeter/trunk/src/jorphan/org/apache/jorphan/exec/KeyToolUtils.java
Date Mon, 09 Sep 2013 10:44:26 GMT
On 8 September 2013 21:48, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> Hello,
> In this case why leave this code:
> //not strictly needed
> new File(CACERT).delete();
>
> As reading it I understand it is not an issue if delete fails? while in
> fact it is ?

The task here is to update the key store and generate a new certificate.
If the gencert cannot create the file, it will fail at that point.
That's why I added the comment that the CACERT delete was not necessary.

I put the deletes in originally partly for tidiness.
Delete the output files at the start, and then recreate them.
Also the CACERT file is useless without the key store.

Question is: do we need to know if the delete failed?

If not, there's no point checking and reporting it.
If we do need to know, then we should probably report failure to
delete the key store as well.

But if we do go down that route, then it's probably safer to use code like:

if (!file.delete()  && file.exists()) {
   // report error
}

>
> On Sun, Sep 8, 2013 at 10:46 PM, sebb <sebbaz@gmail.com> wrote:
>
>> On 8 September 2013 21:39,  <pmouawad@apache.org> wrote:
>> > Author: pmouawad
>> > Date: Sun Sep  8 20:39:35 2013
>> > New Revision: 1520921
>> >
>> > URL: http://svn.apache.org/r1520921
>> > Log:
>> > Test for existence before trying to delete (thanks sebb)
>> >
>> > Modified:
>> >     jmeter/trunk/src/jorphan/org/apache/jorphan/exec/KeyToolUtils.java
>> >
>> > Modified:
>> jmeter/trunk/src/jorphan/org/apache/jorphan/exec/KeyToolUtils.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/jorphan/org/apache/jorphan/exec/KeyToolUtils.java?rev=1520921&r1=1520920&r2=1520921&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/src/jorphan/org/apache/jorphan/exec/KeyToolUtils.java
>> (original)
>> > +++ jmeter/trunk/src/jorphan/org/apache/jorphan/exec/KeyToolUtils.java
>> Sun Sep  8 20:39:35 2013
>> > @@ -118,7 +118,7 @@ public class KeyToolUtils {
>> >      public static void generateProxyCA(File keystore, String password,
>>  int validity) throws IOException {
>> >          keystore.delete(); // any existing entries will be invalidated
>> anyway
>> >          // not strictly needed
>> > -        if(!new File(CACERT).delete()) {
>> > +        if(new File(CACERT).exists() && !new File(CACERT).delete())
{
>>
>> I still think it's better to leave the failure reporting to the gencert
>> command.
>>
>> e.g. if the CACERT file is read-only, we will now get a warning and an
>> error.
>>
>> >              // Noop as we accept not to be able to delete it
>> >              log.warn("Could not delete file:"+new
>> File(CACERT).getAbsolutePath()+", will continue ignoring this");
>> >          }
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message