-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14453/#review26750
-----------------------------------------------------------

Ship it!



src/java/jni/convert.cpp
<https://reviews.apache.org/r/14453/#comment52068>

    != NULL



src/java/jni/convert.cpp
<https://reviews.apache.org/r/14453/#comment52067>

    Spacing issues.



src/java/jni/convert.cpp
<https://reviews.apache.org/r/14453/#comment52070>

    Can we do this once at the beginning of the function?
    
    static jclass NO_SUCH_FIELD_ERROR = NULL;
    
    if (NO_SUCH_FIELD_ERROR == NULL) {
      NO_SUCH_FIELD_ERROR = env->FindClass("java/lang/NoSuchFieldError");
      if (env->ExceptionOccured()) {
        return Error("Cannot find class NoSuchFieldError");
      }
    }
    
    jfieldID field = env->GetFieldID(clazz, name, signature);
    jthrowable jexception = env->ExceptionOccurred();
    if (jexception != NULL) {
      env->ExceptionClear(); // Clear the exception first before proceeding.
      if (!env->IsInstanceOf(jexception, NO_SUCH_FIELD_ERROR)) {        
         // We are here if we got a different exception than
         // 'NoSuchFieldError'. Rethrow and bail.
         env->Throw(jexception);
         return Error("Unexpected exception");
       }
       return None(); // Field doesn't exist.
    }
    
    return field;



src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
<https://reviews.apache.org/r/14453/#comment52071>

    Don't worry about re-throwing, it's already thrown, just return.



src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp
<https://reviews.apache.org/r/14453/#comment52072>

    if (jcredential != NULL) {
    
    Then we can only use one variable and I don't have to reason about whether 
or not it's possible to pass NULL to construct.


- Benjamin Hindman


On Oct. 7, 2013, 8:05 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14453/
> -----------------------------------------------------------
> 
> (Updated Oct. 7, 2013, 8:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-704
>     https://issues.apache.org/jira/browse/MESOS-704
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/convert.hpp 51a93cacf2e8e98f972bda88d8be18f3ad31705e 
>   src/java/jni/convert.cpp b76a5960cc9bd6828886083fb2793482a2b13c3a 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d 
>   src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
> 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84 
> 
> Diff: https://reviews.apache.org/r/14453/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Tested with all different configurations for jar.
> --> old jar which doesn't have credentials field
> --> new jar which sets credentials to null
> --> new jar which sets credentials
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to