I saw you've integrated this.

Just want to add that, instead of mode (which is used also for part of the transformation, e.g. CBC, GCM), maybe we can use "operation" or "operating mode". Something to keep in mind for future changes, I guess.

Regards,
Valerie

On 11/17/2018 11:36 PM, Weijun Wang wrote:
The version looks fine. Thanks.

On Nov 17, 2018, at 1:56 AM, Seán Coffey <sean.cof...@oracle.com> wrote:

Thanks Sean. StringBuilder use added :

http://cr.openjdk.java.net/~coffeys/webrev.8210838.v4/webrev/

Regards,
Sean.

On 16/11/18 17:33, Sean Mullan wrote:
Looks ok. If there are no disadvantages to using a StringBuilder, I would 
probably do that, since you are creating 4-5 separate Strings in the toString 
method.

--Sean

On 11/16/18 11:35 AM, Seán Coffey wrote:
On 16/11/18 16:16, Sean Mullan wrote:
On 11/16/18 9:04 AM, Seán Coffey wrote:
That's a good example and point Max. How does this revision look ?

http://cr.openjdk.java.net/~coffeys/webrev.8210838.v2/webrev/
2832      * This implementation returns a String containing the transformation
2833      * used by this Cipher, the Cipher mode and the Cipher Provider.

I would suggest rewording this as: "This implementation returns a String containing 
the transformation, mode, and provider of this Cipher."
Good suggestion. Changed.
2840         String m = "not initialized";
2841         if (initialized)
2842             m = getOpmodeString(opmode);

Or:

String m = initialized ? getOpmodeString(opmode) : "not initialized";
I've moved the switch expression into the toString() call now as per suggestion 
from Max. I've just
seen that we should never have an unexpected opmode given the checkOpmode(int) 
method check.
I've let an unexpected opmode register with "error" in the switch statement. // 
should never happen.

webrev updated in place:
http://cr.openjdk.java.net/~coffeys/webrev.8210838.v3/webrev/

regards,
Sean.

Also, it might be worthwhile to use a StringBuilder if you think this method 
may be called often.

--Sean

Regards,
Sean.

On 16/11/18 03:35, Weijun Wang wrote:
Signature's toString looks like

public String toString() {
    String initState = "";
    switch (state) {
    case UNINITIALIZED:
        initState = "<not initialized>";
        break;
    case VERIFY:
        initState = "<initialized for verifying>";
        break;
    case SIGN:
        initState = "<initialized for signing>";
        break;
    }
    return "Signature object: " + getAlgorithm() + initState;
}

Maybe you can add some similar info.

In fact, it looks like you can extract the debug output at the end of each 
init() methods into a new toString() method.

Thanks
Max

On Nov 16, 2018, at 12:35 AM, Seán Coffey <sean.cof...@oracle.com> wrote:

A simple enhancement to override toString() for javax.crypto.Cipher class

https://bugs.openjdk.java.net/browse/JDK-8210838
webrev : http://cr.openjdk.java.net/~coffeys/webrev.8210838/webrev/

regards,
Sean.


Reply via email to