zookeeper-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [zookeeper] enixon commented on a change in pull request #999: ZOOKEEPER-2891: Invalid processing of zookeeper_close for mutli-request
Date Mon, 24 Jun 2019 17:24:48 GMT
enixon commented on a change in pull request #999: ZOOKEEPER-2891: Invalid processing of zookeeper_close
for mutli-request
URL: https://github.com/apache/zookeeper/pull/999#discussion_r296830501
 
 

 ##########
 File path: zookeeper-client/zookeeper-client-c/src/zookeeper.c
 ##########
 @@ -2010,24 +2010,28 @@ static void process_sync_completion(
     case COMPLETION_VOID:
         break;
     case COMPLETION_MULTI:
-        sc->rc = deserialize_multi(cptr->xid, cptr, ia);
+        sc->rc = deserialize_multi(cptr->xid, cptr, ia, sc->rc);
         break;
     default:
         LOG_DEBUG(("Unsupported completion type=%d", cptr->c.type));
         break;
     }
 }
 
-static int deserialize_multi(int xid, completion_list_t *cptr, struct iarchive *ia)
+static int deserialize_multi(int xid, completion_list_t *cptr, struct iarchive *ia, int rc_hint)
 {
-    int rc = 0;
+    int rc = rc_hint;
     completion_head_t *clist = &cptr->c.clist;
     struct MultiHeader mhdr = { STRUCT_INITIALIZER(type , 0), STRUCT_INITIALIZER(done , 0),
STRUCT_INITIALIZER(err , 0) };
     assert(clist);
     deserialize_MultiHeader(ia, "multiheader", &mhdr);
     while (!mhdr.done) {
         completion_list_t *entry = dequeue_completion(clist);
-        assert(entry);
+        if (rc_hint == ZOK) {
+            assert(entry);
+        } else if (entry == NULL) {
+            break;
 
 Review comment:
   Can you explain why this is the right point to exit the loop and the method? To my eyes,
it is cleaner to have `rc_hint == ZOK` as a conditional on the `while` statement or perhaps
a `return rc` down a few lines where it is set on error. 
   
   Could we also return immediately from the method is `rc_hint` is not 0?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message