code review request: 7081783: jarsigner error when no $HOME/.keystore

[Xuelei Fan]

On 9/1/2011 12:31 PM, Weijun Wang wrote:
> 
> On 08/31/2011 09:17 PM, Xuelei Fan wrote:
>> I understand the code, fine to me. But the loadKeyStore() method looks
>> really ugly and lazy. :-)
> 
> It's ugly, but not very lazy.
> 
Lazy code is not a bad coding style. ;-)

> Anyway, I'm going to putback this version since you already said fine.
> 
Please go ahead. It's fine to me. I just want to talk about some of my
concerns.

Thanks,
Xuelei

>>
>> Just for your reference in the inline comments.
>>
> ...
> 
>>>
>>> Well, it looks more correct, but is complicated in 2 senses:
>>>
>>> 1. ~/.keystore and user-specified -keystore are not treated the same.
>>> You can ignore ~/.keystore, but if a user-specified -keystore does not
>>> exist, it's an error.
>>>
>> We can handle the logic simply at the following blok, right?
>> 1562 if (!nullStream&&  keyStoreName == null) {
>> 1563     keyStoreName = System.getProperty("user.home") + File.separator
>> 1564         + ".keystore";
>> +        // check file existence, ignore it if non-exist
>> 1565 }
>>
>>> 2. signing and verification have different behaviors on exception
>>> handling. See above.
>>>
>> We don't need to make more significant update in other blocks because of
>> we have ignore the non-exist keystore, right? If no required private
>> key, the following steps will throw the expected exception.
> 
> If you ignore non-existing ~/.keystore, there will be no exception, and
> store will be either null or uninitialized. Then when it's used, NPE or
> KeyStoreException will be thrown and the user is confused. So you still
> need to check it earlier.
> 
> Thanks
> Max
> 
>>
>> Or is there any other thing that I missed?
>>
>> Thanks,
>> Xuelei