Refactor command and query execute functions

This corrects the function signatures to return a boolean indicating success instead of an integer (which was cast from a boolean).

Additionally, it restructures phongo_execute_command() to use a cleanup label, in anticipation of implicit session changes.

Author: jmikola

php_phongo.c

@@ -749,7 +749,7 @@ bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t *cursor TSRMLS_DC
 	return true;
 } /* }}} */
 
-int phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *zquery, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
+bool phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *zquery, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
 {
 	const php_phongo_query_t *query;
 	mongoc_cursor_t *cursor;
@@ -827,7 +827,7 @@ static bson_t *create_wrapped_command_envelope(const char *db, bson_t *reply)
 	return tmp;
 }
 
-int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
+bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
 {
 	const php_phongo_command_t *command;
 	bson_iter_t iter;
@@ -837,38 +837,34 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
 	mongoc_cursor_t *cmd_cursor;
 	zval *zreadPreference = NULL;
 	zval *zsession = NULL;
-	int result;
+	bool result = false;
+	bool free_reply = false;
 
 	command = Z_COMMAND_OBJ_P(zcommand);
 
 	if ((type & PHONGO_OPTION_READ_CONCERN) && !phongo_parse_read_concern(options, &opts TSRMLS_CC)) {
 		/* Exception should already have been thrown */
-		bson_destroy(&opts);
-		return false;
+		goto cleanup;
 	}
 
 	if ((type & PHONGO_OPTION_READ_PREFERENCE) && !phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) {
 		/* Exception should already have been thrown */
-		bson_destroy(&opts);
-		return false;
+		goto cleanup;
 	}
 
 	if (!phongo_parse_session(options, client, &opts, &zsession TSRMLS_CC)) {
 		/* Exception should already have been thrown */
-		bson_destroy(&opts);
-		return false;
+		goto cleanup;
 	}
 
 	if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) {
 		/* Exception should already have been thrown */
-		bson_destroy(&opts);
-		return false;
+		goto cleanup;
 	}
 
 	if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) {
 		phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option");
-		bson_destroy(&opts);
-		return false;
+		goto cleanup;
 	}
 
 	/* Although "opts" already always includes the serverId option, the read
@@ -891,21 +887,18 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
 		default:
 			/* Should never happen, but if it does: exception */
 			phongo_throw_exception(PHONGO_ERROR_LOGIC TSRMLS_CC, "Type '%d' should never have been passed to phongo_execute_command, please file a bug report", type);
-			bson_destroy(&opts);
-			return false;
+			goto cleanup;
 	}
+
+	free_reply = true;
+
 	if (!result) {
 		phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
-		bson_destroy(&reply);
-		bson_destroy(&opts);
-		return false;
+		goto cleanup;
 	}
 
-	bson_destroy(&opts);
-
 	if (!return_value_used) {
-		bson_destroy(&reply);
-		return true;
+		goto cleanup;
 	}
 
 	/* According to mongoc_cursor_new_from_command_reply(), the reply bson_t
@@ -926,16 +919,22 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
 		}
 
 		cmd_cursor = mongoc_cursor_new_from_command_reply(client, &initial_reply, server_id);
-		bson_destroy(&reply);
 	} else {
 		bson_t *wrapped_reply = create_wrapped_command_envelope(db, &reply);
 
 		cmd_cursor = mongoc_cursor_new_from_command_reply(client, wrapped_reply, server_id);
-		bson_destroy(&reply);
 	}
 
 	phongo_cursor_init_for_command(return_value, client, cmd_cursor, db, zcommand, zreadPreference, zsession TSRMLS_CC);
-	return true;
+
+cleanup:
+	bson_destroy(&opts);
+
+	if (free_reply) {
+		bson_destroy(&reply);
+	}
+
+	return result;
 } /* }}} */
 /* }}} */
 

php_phongo.h

@@ -134,8 +134,8 @@ void                     phongo_readconcern_init     (zval *return_value, const
 void                     phongo_readpreference_init  (zval *return_value, const mongoc_read_prefs_t *read_prefs TSRMLS_DC);
 void                     phongo_writeconcern_init    (zval *return_value, const mongoc_write_concern_t *write_concern TSRMLS_DC);
 bool                     phongo_execute_bulk_write   (mongoc_client_t *client, const char *namespace, php_phongo_bulkwrite_t *bulk_write, zval *zwriteConcern, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
-int                      phongo_execute_command      (mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
-int                      phongo_execute_query        (mongoc_client_t *client, const char *namespace, zval *zquery, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
+bool                     phongo_execute_command      (mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
+bool                     phongo_execute_query        (mongoc_client_t *client, const char *namespace, zval *zquery, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
 
 bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t *cursor TSRMLS_DC);