You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by el...@apache.org on 2015/02/01 10:29:06 UTC

directory-kerberos git commit: Improved the Javadoc, added some missing parts.

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/68b5f1bb
Tree: http://git-wip-us.apache.org/repos/asf/directory-kerberos/tree/68b5f1bb
Diff: http://git-wip-us.apache.org/repos/asf/directory-kerberos/diff/68b5f1bb

Branch: refs/heads/master
Commit: 68b5f1bb278656f4501899dbbf8776f05124bef0
Parents: c69f385
Author: Emmanuel Lécharny <el...@symas.com>
Authored: Sun Feb 1 10:28:53 2015 +0100
Committer: Emmanuel Lécharny <el...@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/68b5f1bb/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;


RE: directory-kerberos git commit: Improved the Javadoc, added some missing parts.

Posted by "Zheng, Kai" <ka...@intel.com>.
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 <el...@symas.com>
> Authored: Sun Feb 1 10:28:53 2015 +0100
> Committer: Emmanuel Lécharny <el...@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;
>


Re: directory-kerberos git commit: Improved the Javadoc, added some missing parts.

Posted by Emmanuel Lécharny <el...@gmail.com>.
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/68b5f1bb
> Tree: http://git-wip-us.apache.org/repos/asf/directory-kerberos/tree/68b5f1bb
> Diff: http://git-wip-us.apache.org/repos/asf/directory-kerberos/diff/68b5f1bb
>
> Branch: refs/heads/master
> Commit: 68b5f1bb278656f4501899dbbf8776f05124bef0
> Parents: c69f385
> Author: Emmanuel Lécharny <el...@symas.com>
> Authored: Sun Feb 1 10:28:53 2015 +0100
> Committer: Emmanuel Lécharny <el...@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/68b5f1bb/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;
>