collection: followup api cleanup for mongoc_collection_get_last_error().

This patches changes the semantics a bit of Maga's patch for
mongoc_collection_get_last_error(). It uses the new bson_clear() macro
to clear the value of collection->gle where appropriate. Previously,
it was not setting the value to NULL after destroying.

Additionally, we remove the concept of collection from the mongoc_client_t
code and simply use an output parameter for the document. This may be useful
for situations like gridfs or database if we want to directly communicate
with the client layer.

The return value from mongoc_collection_get_last_error() is now a const,
and we rely on the caller to copy. This let's us avoid extra copies unless
they are necessary by the caller.

Author: chergert

doc/mongoc_collection_get_last_error.txt

@@ -1,5 +1,5 @@
 mongoc_collection_get_last_error(3)
-=============================
+===================================
 
 
 NAME
@@ -11,7 +11,7 @@ SYNOPSIS
 --------
 [source,c]
 -----------------------
-bson_t *
+const bson_t *
 mongoc_collection_get_last_error (const mongoc_collection_t *collection);
 -----------------------
 
@@ -36,11 +36,11 @@ _mongoc_collection_ensure_index()_
 
 RETURN VALUE
 ------------
-A newly allocated bson_t getLastError document, that *must* be destroyed
-with _bson_destroy()_. Or NULL if no getLastError present.
+A pointer to a bson_t that must not be modified or freed. This document is not
+guaranteed to be valid between calls into mongoc_collection_t. Therefore, you
+must call bson_copy() if you want to hold on to it between API calls.
 
 
 AUTHORS
 -------
-
-This page was written by MongoDB Inc.
+This page was written by MongoDB, Inc.

src/mongoc/mongoc-client-private.h

@@ -74,8 +74,9 @@ bool             _mongoc_client_recv          (mongoc_client_t              *cli
                                                mongoc_buffer_t              *buffer,
                                                uint32_t                      hint,
                                                bson_error_t                 *error);
-bool             _mongoc_client_recv_gle      (mongoc_collection_t          *collection,
+bool             _mongoc_client_recv_gle      (mongoc_client_t              *client,
                                                uint32_t                      hint,
+                                               bson_t                      **gle_doc,
                                                bson_error_t                 *error);
 uint32_t         _mongoc_client_stamp         (mongoc_client_t              *client,
                                                uint32_t                      node);

src/mongoc/mongoc-client.c

@@ -543,22 +543,27 @@ _bson_to_error (const bson_t *b,
  *
  *       If the operation was successful, true is returned.
  *
+ *       if @gle_doc is not NULL, then the actual response document for
+ *       the gle command will be stored as an out parameter. The caller
+ *       is responsible for freeing it in this case.
+ *
  * Returns:
  *       true if getlasterror was success; otherwise false and @error
  *       is set.
  *
  * Side effects:
  *       @error if return value is false.
+ *       @gle_doc will be set if non NULL and a reply was received.
  *
  *--------------------------------------------------------------------------
  */
 
 bool
-_mongoc_client_recv_gle (mongoc_collection_t    *collection,
-                         uint32_t    hint,
-                         bson_error_t    *error)
+_mongoc_client_recv_gle (mongoc_client_t  *client,
+                         uint32_t          hint,
+                         bson_t          **gle_doc,
+                         bson_error_t     *error)
 {
-   mongoc_client_t *client;
    mongoc_buffer_t buffer;
    mongoc_rpc_t rpc;
    bson_iter_t iter;
@@ -567,11 +572,13 @@ _mongoc_client_recv_gle (mongoc_collection_t    *collection,
 
    ENTRY;
 
-   bson_return_val_if_fail (collection, false);
-   client = collection->client;
    bson_return_val_if_fail (client, false);
    bson_return_val_if_fail (hint, false);
 
+   if (gle_doc) {
+      *gle_doc = NULL;
+   }
+
    _mongoc_buffer_init (&buffer, NULL, 0, NULL);
 
    if (!_mongoc_cluster_try_recv (&client->cluster, &rpc, &buffer,
@@ -587,25 +594,24 @@ _mongoc_client_recv_gle (mongoc_collection_t    *collection,
       GOTO (cleanup);
    }
 
-   if ((rpc.reply.flags & MONGOC_REPLY_QUERY_FAILURE)) {
-      if (_mongoc_rpc_reply_get_first (&rpc.reply, &b)) {
+   if (_mongoc_rpc_reply_get_first (&rpc.reply, &b)) {
+      if (gle_doc) {
+         *gle_doc = bson_copy (&b);
+      }
+
+      if ((rpc.reply.flags & MONGOC_REPLY_QUERY_FAILURE)) {
          _bson_to_error (&b, error);
          bson_destroy (&b);
          GOTO (cleanup);
       }
-   }
 
-   if (_mongoc_rpc_reply_get_first (&rpc.reply, &b)) {
       if (!bson_iter_init_find (&iter, &b, "ok") ||
           BSON_ITER_HOLDS_DOUBLE (&iter)) {
         if (bson_iter_double (&iter) == 0.0) {
           _bson_to_error (&b, error);
         }
-        if (collection->gle) {
-          bson_destroy (collection->gle);
-        }
-        collection->gle = bson_copy (&b);
       }
+
       bson_destroy (&b);
    }
 

src/mongoc/mongoc-collection.c

@@ -129,9 +129,7 @@ mongoc_collection_destroy (mongoc_collection_t *collection) /* IN */
 
    bson_return_if_fail(collection);
 
-   if (collection->gle) {
-      bson_destroy(collection->gle);
-   }
+   bson_clear (&collection->gle);
 
    _mongoc_buffer_destroy(&collection->buffer);
 
@@ -295,14 +293,12 @@ mongoc_collection_find (mongoc_collection_t       *collection, /* IN */
    bson_return_val_if_fail(collection, NULL);
    bson_return_val_if_fail(query, NULL);
 
+   bson_clear (&collection->gle);
+
    if (!read_prefs) {
       read_prefs = collection->read_prefs;
    }
 
-   if (collection->gle) {
-      bson_destroy(collection->gle);
-   }
-
    return _mongoc_cursor_new(collection->client, collection->ns, flags, skip,
                              limit, batch_size, false, query, fields, read_prefs);
 }
@@ -354,9 +350,7 @@ mongoc_collection_command (mongoc_collection_t       *collection,
       read_prefs = collection->read_prefs;
    }
 
-   if (collection->gle) {
-      bson_destroy(collection->gle);
-   }
+   bson_clear (&collection->gle);
 
    return mongoc_client_command (collection->client, collection->db, flags,
                                  skip, limit, batch_size, query, fields, read_prefs);
@@ -372,9 +366,7 @@ mongoc_collection_command_simple (mongoc_collection_t       *collection,
    BSON_ASSERT (collection);
    BSON_ASSERT (command);
 
-   if (collection->gle) {
-      bson_destroy(collection->gle);
-   }
+   bson_clear (&collection->gle);
 
    return mongoc_client_command_simple (collection->client, collection->db,
                                         command, read_prefs, reply, error);
@@ -680,6 +672,8 @@ _mongoc_collection_insert_bulk_raw (mongoc_collection_t          *collection,
    BSON_ASSERT(documents);
    BSON_ASSERT(n_documents);
 
+   bson_clear (&collection->gle);
+
    if (!write_concern) {
       write_concern = collection->write_concern;
    }
@@ -740,9 +734,6 @@ _mongoc_collection_insert_bulk_raw (mongoc_collection_t          *collection,
       bson_init_static (&reply_bson, reply.reply.documents,
                         reply.reply.documents_len);
 
-      if (collection->gle) {
-         bson_destroy (collection->gle);
-      }
       collection->gle = bson_copy (&reply_bson);
 
       if (bson_iter_init_find (&reply_iter, &reply_bson, "err") &&
@@ -818,6 +809,8 @@ mongoc_collection_insert_bulk (mongoc_collection_t           *collection,
    BSON_ASSERT (documents);
    BSON_ASSERT (n_documents);
 
+   bson_clear (&collection->gle);
+
    if (!(flags & MONGOC_INSERT_NO_VALIDATE)) {
       for (i = 0; i < n_documents; i++) {
          if (!bson_validate (documents [i],
@@ -901,6 +894,8 @@ mongoc_collection_insert (mongoc_collection_t          *collection,
    bson_return_val_if_fail (collection, false);
    bson_return_val_if_fail (document, false);
 
+   bson_clear (&collection->gle);
+
    if (!bson_iter_init_find (&iter, document, "_id")) {
       bson_oid_init (&oid, NULL);
       bson_append_oid (&copy, "_id", 3, &oid);
@@ -961,6 +956,8 @@ mongoc_collection_update (mongoc_collection_t          *collection,
    bson_return_val_if_fail(selector, false);
    bson_return_val_if_fail(update, false);
 
+   bson_clear (&collection->gle);
+
    if (!(flags & MONGOC_UPDATE_NO_VALIDATE) &&
        bson_iter_init (&iter, update) &&
        bson_iter_next (&iter) &&
@@ -1005,7 +1002,8 @@ mongoc_collection_update (mongoc_collection_t          *collection,
    }
 
    if (_mongoc_write_concern_has_gle (write_concern)) {
-      if (!_mongoc_client_recv_gle (collection, hint, error)) {
+      if (!_mongoc_client_recv_gle (collection->client, hint,
+                                    &collection->gle, error)) {
          RETURN(false);
       }
    }
@@ -1113,6 +1111,8 @@ mongoc_collection_delete (mongoc_collection_t          *collection,
    bson_return_val_if_fail(collection, false);
    bson_return_val_if_fail(selector, false);
 
+   bson_clear (&collection->gle);
+
    if (!write_concern) {
       write_concern = collection->write_concern;
    }
@@ -1136,7 +1136,8 @@ mongoc_collection_delete (mongoc_collection_t          *collection,
    }
 
    if (_mongoc_write_concern_has_gle(write_concern)) {
-      if (!_mongoc_client_recv_gle (collection, hint, error)) {
+      if (!_mongoc_client_recv_gle (collection->client, hint,
+                                    &collection->gle, error)) {
          return false;
       }
    }
@@ -1294,19 +1295,21 @@ mongoc_collection_get_name (mongoc_collection_t *collection)
  *       executed command for current collection instance.
  *
  * Returns:
- *       A newly allocated bson_t getLastError document, that *must* be
- *       destroyed with bson_destroy(). Or NULL if no getLastError present.
+ *       NULL or a bson_t that should not be modified or freed. This value
+ *       is not guaranteed to be persistent between calls into the
+ *       mongoc_collection_t instance, and therefore must be copied if
+ *       you would like to keep it around.
  *
  * Side effects:
  *       None.
  *
  *--------------------------------------------------------------------------
  */
 
-bson_t *
-mongoc_collection_get_last_error (const mongoc_collection_t *collection)
+const bson_t *
+mongoc_collection_get_last_error (const mongoc_collection_t *collection) /* IN */
 {
    bson_return_val_if_fail (collection, NULL);
 
-   return collection->gle ? bson_copy (collection->gle) : NULL;
+   return collection->gle;
 }

src/mongoc/mongoc-collection.h

@@ -114,7 +114,7 @@ const mongoc_write_concern_t *mongoc_collection_get_write_concern    (const mong
 void                          mongoc_collection_set_write_concern    (mongoc_collection_t           *collection,
                                                                       const mongoc_write_concern_t  *write_concern);
 const char                   *mongoc_collection_get_name             (mongoc_collection_t           *collection);
-bson_t                       *mongoc_collection_get_last_error       (const mongoc_collection_t *collection);
+const bson_t                 *mongoc_collection_get_last_error       (const mongoc_collection_t     *collection);
 char                         *mongoc_collection_keys_to_index_string (const bson_t                  *keys);