RFR: 7903020: Command line error: Incorrect output filename - can't write

[Alexandre Iline]

On Mon, 30 Aug 2021 18:40:08 GMT, Leonid Kuskov <lkuskov at openjdk.org> wrote:

>> src/classes/com/sun/tdk/jcov/Merger.java line 1017:
>> 
>>> 1015:         template = opts.getValue(DSC_TEMPLATE);
>>> 1016:         Utils.checkFileCanBeNull(template, "template filename",
>>> 1017:                 Utils.CheckOptions.FILE_EXISTS, Utils.CheckOptions.FILE_ISFILE, Utils.CheckOptions.FILE_CANREAD);
>> 
>> is FILE_EXISTS a subset of FILE_CANREAD?
>
> Yes, it is. Actually there are 20 places where this check is used. 
> Here FILE_CANREAD covers both EXISTS && ISFILE

if FILE_CANREAD is stronger than both other checks, should it be the only permission listed?

>> src/classes/com/sun/tdk/jcov/Merger.java line 1022:
>> 
>>> 1020: 
>>> 1021:         output = opts.getValue(DSC_OUTPUT);
>>> 1022:         Utils.checkFileNotNull(output, "output filename",
>> 
>> It appears that the set of permissions is the same but in a different order.
>> It would help if the changset did not contain any formatting changes
>
> OK, I just truncated code lines according to the java code style and changed the order of checking to get more natural list of reasons for failure. Do you need to restore the "old" fashion?

It's not that important. I am just saying that the purely formatting changes make it harder to make sure that the fix is correct.

>> src/classes/com/sun/tdk/jcov/util/Utils.java line 1192:
>> 
>>> 1190:                     }
>>> 1191:                     break;
>>> 1192:                 case FILE_CANWRITE:
>> 
>> I assume you have reviewed all usages of FILE_CANWRITE, because by this change you do change the meaning of it.
>
> Yes, right now the CANWRITE is wider that just writing into existing file is allowed. Here it gives write permissions to non-existing file in existing directory that has write permissions

My question was whether you checked that there are no places in JCov code where CAN_WRITE is used to also make sure that the file exists.

-------------

PR: https://git.openjdk.java.net/jcov/pull/18