directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zheng, Kai" <kai.zh...@intel.com>
Subject RE: directory-kerberos git commit: Improved the Javadoc, added some missing parts.
Date Sun, 01 Feb 2015 21:04:26 GMT
Hi Emmanuel,

>>making the hex2bytes() method as fast as possible, removing the call to split() and
char[] copy, 
It's great to have the optimization, though it's currently only for test codes but I guess
we may need it in production codes in future.

>>but at the same time, the main loop is not optimal, as I had to add a test to get
rid of the 2 first chars
Maybe we can remove the 2 first chars from our test data so won't bother on them at all. 

Sorry but I have to tell you that there're similar copy of it in kerby-util module. It's for
all the projects. I'm not sure if we'd better or not
Consolidate them into one, because kerby-asn1 not relying on kerby-util may be a good idea.
How do you think ?

Regards,
Kai

-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com] 
Sent: Sunday, February 01, 2015 5:35 PM
To: dev@directory.apache.org
Subject: Re: directory-kerberos git commit: Improved the Javadoc, added some missing parts.

Hi Kerby fellows,

I have made a small mistake, pushing some code that was not intended to be pushed. Typically,
this modification of the Util class was done locally for the fun only.

We can keep it, but I would really don't mind if we come back to the original code which is
way more dense.

(I played around the idea of making the hex2bytes() method as fast as possible, removing the
call to split() and char[] copy, but at the same time, the main loop is not optimal, as I
had to add a test to get rid of the 2 first chars, something that could have been improved
using a for (int i = 2; i<hexStr.length; i++))

So let me know, I can revert it back to what it was before.

Sorry for the noise...

Le 01/02/15 10:29, elecharny@apache.org a écrit :
> Repository: directory-kerberos
> Updated Branches:
>   refs/heads/master c69f3853b -> 68b5f1bb2
>
>
> Improved the Javadoc, added some missing parts.
>
> Project: 
> http://git-wip-us.apache.org/repos/asf/directory-kerberos/repo
> Commit: 
> http://git-wip-us.apache.org/repos/asf/directory-kerberos/commit/68b5f
> 1bb
> Tree: 
> http://git-wip-us.apache.org/repos/asf/directory-kerberos/tree/68b5f1b
> b
> Diff: 
> http://git-wip-us.apache.org/repos/asf/directory-kerberos/diff/68b5f1b
> b
>
> Branch: refs/heads/master
> Commit: 68b5f1bb278656f4501899dbbf8776f05124bef0
> Parents: c69f385
> Author: Emmanuel Lécharny <elecharny@symas.com>
> Authored: Sun Feb 1 10:28:53 2015 +0100
> Committer: Emmanuel Lécharny <elecharny@symas.com>
> Committed: Sun Feb 1 10:28:53 2015 +0100
>
> ----------------------------------------------------------------------
>  .../test/java/org/apache/kerby/asn1/Util.java   | 105 ++++++++++++++++---
>  1 file changed, 93 insertions(+), 12 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/directory-kerberos/blob/68b5f1b
> b/kerby-asn1/src/test/java/org/apache/kerby/asn1/Util.java
> ----------------------------------------------------------------------
> diff --git a/kerby-asn1/src/test/java/org/apache/kerby/asn1/Util.java 
> b/kerby-asn1/src/test/java/org/apache/kerby/asn1/Util.java
> index 5f58e1a..446917a 100644
> --- a/kerby-asn1/src/test/java/org/apache/kerby/asn1/Util.java
> +++ b/kerby-asn1/src/test/java/org/apache/kerby/asn1/Util.java
> @@ -50,21 +50,102 @@ public class Util {
>       * 0x02 02 00 80
>       */
>      public static byte[] hex2bytes(String hexString) {
> -        hexString = hexString.toUpperCase();
> -        String hexStr = hexString;
> -        if (hexString.startsWith("0X")) {
> -            hexStr = hexString.substring(2);
> +        if (hexString==null) {
> +            throw new IllegalArgumentException("Invalid hex string to 
> + convert : null");
>          }
> -        String[] hexParts = hexStr.split(" ");
> +        
> +        char[] hexStr = hexString.toCharArray();
>  
> -        byte[] bytes = new byte[hexParts.length];
> -        char[] hexPart;
> -        for (int i = 0; i < hexParts.length; ++i) {
> -            hexPart = hexParts[i].toCharArray();
> -            if (hexPart.length != 2) {
> -                throw new IllegalArgumentException("Invalid hex string to convert");
> +        if (hexStr.length < 4) {
> +            throw new IllegalArgumentException("Invalid hex string to convert : length
below 4");
> +        }
> +        
> +        if (( hexStr[0] != '0') || ((hexStr[1]!='x') && (hexStr[1]!='X'))) {
> +            throw new IllegalArgumentException("Invalid hex string to convert : not
starting with '0x'");
> +        }
> +        
> +        byte[] bytes = new byte[(hexStr.length - 1)/3];
> +        int pos = 0; 
> +        boolean high = false;
> +        boolean prefix = true;
> +        
> +        for (char c:hexStr) {
> +            if (prefix) {
> +                if ((c == 'x')|| (c=='X')) {
> +                    prefix = false;
> +                }
> +                
> +                continue;
>              }
> -            bytes[i] = (byte) ((HEX_CHARS_STR.indexOf(hexPart[0]) << 4) + HEX_CHARS_STR.indexOf(hexPart[1]));
> +            
> +            switch (c) {
> +                case ' ' :
> +                    if (high) {
> +                        // We have had only the high part
> +                        throw new IllegalArgumentException("Invalid hex string to convert");
> +                    }
> +                    
> +                    // A hex pair has been decoded
> +                    pos++;
> +                    high = false;
> +                    break;
> +                    
> +                case '0': 
> +                case '1': 
> +                case '2': 
> +                case '3': 
> +                case '4':
> +                case '5': 
> +                case '6':
> +                case '7':
> +                case '8':
> +                case '9':
> +                    if (high) {
> +                        bytes[pos] += (byte)(c - '0');
> +                    } else {
> +                        bytes[pos] = (byte)((c - '0') << 4);
> +                    }
> +                    
> +                    high = !high;
> +                    break;
> +                    
> +                case 'a' :
> +                case 'b' :
> +                case 'c' :
> +                case 'd' :
> +                case 'e' :
> +                case 'f' :
> +                    if (high) {
> +                        bytes[pos] += (byte)(c - 'a' + 10);
> +                    } else {
> +                        bytes[pos] = (byte)((c - 'a' + 10) << 4);
> +                    }
> +
> +                    high = !high;
> +                    break;
> +
> +                case 'A' :
> +                case 'B' :
> +                case 'C' :
> +                case 'D' :
> +                case 'E' :
> +                case 'F' :
> +                    if (high) {
> +                        bytes[pos] += (byte)(c - 'A' + 10);
> +                    } else {
> +                        bytes[pos] = (byte)((c - 'A' + 10) << 4);
> +                    }
> +
> +                    high = !high;
> +                    break;
> +                    
> +                default :
> +                    throw new IllegalArgumentException("Invalid hex string to convert");
> +            }
> +        }
> +        
> +        if (high) {
> +            throw new IllegalArgumentException("Invalid hex string to 
> + convert");
>          }
>  
>          return bytes;
>

Mime
View raw message