[Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

[Xuelei Fan]

> Please review these JNDI changes.
> Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353
> webrev: http://cr.openjdk.java.net/~mduigou/7072353/0/webrev/

Thanks for your effort to make JNDI free of compile-warning. The work is
hard, I appreciate it.

1. I noticed the copyright date of a few files are unchanged, please
update them before you push the changes.

2. src/share/classes/com/sun/jndi/cosnaming/CNCtx.java
   In javax.naming.Context,  Hashtable<?,?> is used for context
environment. In this changes, you use Hashtable<String, java.lang.Object>.
   What do you think if we keep the type of K and V the same as
Hashtable<?,?>?

   I also noticed the similar inconsistency at:
   . com/sun/jndi/dns/DnsContext.java:
       50   Hashtable<Object,Object> environment;
   . com/sun/jndi/ldap/LdapCtx.java
       226  Hashtable<String, java.lang.Object> envprops = null;

3. src/share/classes/com/sun/jndi/dns/DnsContext.java
   What do you think if we have BaseNameClassPairEnumeration to
implement the NamingEnumeration, so that we can share the code of
nextElement()?

  class BaseNameClassPairEnumeration implements NamingEnumeration<T>

*** com/sun/jndi/ldap/Connection.java
 251         } catch (ClassNotFoundException |
 252                  InstantiationException |
 253                  InvocationTargetException |
 254                  IllegalAccessException e) {

I like this new try-catch feature!

4. com/sun/jndi/ldap/LdapCtx.java
1194    return (NamingEnumeration)
1195        new LdapBindingEnumeration(this, answer, name, cont);

   LdapBindingEnumeration is of type NamingEnumeration, it may be not
necessary to convert to NamingEnumeration. Do you mean
NamingEnumeration<Binding>?

       return (NamingEnumeration<Binding>)
           new LdapBindingEnumeration(this, answer, name, cont);

2244  switch (propName) {
   Do you want to indent this block? We usually indent a block even for
switch blocks.

3017 Vector<Object> temp = (Vector)extractURLs(res.errorMessage);
   You may not need the conversion any more, the return value of
extractURLs() has been updated to
   2564     private static Vector<String> extractURLs(String refString)

5. com/sun/jndi/ldap/LdapBindingEnumeration.java
   Why it is necessary to convert the return type twice (line 92)?

   92   return (LdapBindingEnumeration)(NamingEnumeration)
   93           refCtx.listBindings(listArg);

   It's great to use convariant return type. I would suggest add
override tag to make it easy understood.

I am only able to review a quarter of the update today, will continue
tomorrow.

Thanks,
Xuelei