Skip to content

CDRIVER-5756 Coverity fixes #1867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 5 additions & 21 deletions src/common/src/common-b64.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

#include <bson/bson.h>
#include <common-b64-private.h>
#include <common-thread-private.h>
#include <mlib/config.h>
#include <mlib/loop.h>

Expand Down Expand Up @@ -260,24 +261,7 @@ static const uint8_t mongoc_b64rmap_space = 0xfe;
static const uint8_t mongoc_b64rmap_invalid = 0xff;

/* initializing the reverse map isn't thread safe, do it in pthread_once */
#if defined(BSON_OS_UNIX)
#include <pthread.h>
#define mongoc_common_once_t pthread_once_t
#define mongoc_common_once pthread_once
#define MONGOC_COMMON_ONCE_FUN(n) void n (void)
#define MONGOC_COMMON_ONCE_RETURN return
#define MONGOC_COMMON_ONCE_INIT PTHREAD_ONCE_INIT
#else
#define mongoc_common_once_t INIT_ONCE
#define MONGOC_COMMON_ONCE_INIT INIT_ONCE_STATIC_INIT
#define mongoc_common_once(o, c) InitOnceExecuteOnce (o, c, NULL, NULL)
#define MONGOC_COMMON_ONCE_FUN(n) \
BOOL CALLBACK MLIB_PRAGMA_IF_MSVC (warning (push)) MLIB_PRAGMA_IF_MSVC (warning (disable : 4100)) \
n (PINIT_ONCE _ignored_a, PVOID _ignored_b, PVOID *_ignored_c) MLIB_PRAGMA_IF_MSVC (warning (pop))
#define MONGOC_COMMON_ONCE_RETURN return true
#endif

static MONGOC_COMMON_ONCE_FUN (bson_b64_initialize_rmap)
static BSON_ONCE_FUN (bson_b64_initialize_rmap)
{
/* Null: end of string, stop parsing */
mongoc_b64rmap[0] = mongoc_b64rmap_end;
Expand All @@ -299,7 +283,7 @@ static MONGOC_COMMON_ONCE_FUN (bson_b64_initialize_rmap)
for (uint8_t i = 0; Base64[i] != '\0'; ++i)
mongoc_b64rmap[(uint8_t) Base64[i]] = i;

MONGOC_COMMON_ONCE_RETURN;
BSON_ONCE_RETURN;
}

static int
Expand Down Expand Up @@ -514,9 +498,9 @@ mongoc_b64_pton_len (char const *src)
int
mcommon_b64_pton (char const *src, uint8_t *target, size_t targsize)
{
static mongoc_common_once_t once = MONGOC_COMMON_ONCE_INIT;
static bson_once_t once = BSON_ONCE_INIT;

mongoc_common_once (&once, bson_b64_initialize_rmap);
bson_once (&once, bson_b64_initialize_rmap);

if (!src) {
return -1;
Expand Down
20 changes: 12 additions & 8 deletions src/libbson/src/bson/bson-json.c
Original file line number Diff line number Diff line change
Expand Up @@ -347,13 +347,17 @@ _noop (void)
bson->code_data.in_scope = false; \
} while (0)
#define STACK_POP_DBPOINTER STACK_POP_DOC (_noop ())
#define BASIC_CB_PREAMBLE \
const char *key; \
size_t len; \
bson_json_reader_bson_t *bson = &reader->bson; \
_bson_json_read_fixup_key (bson); \
key = bson->key; \
len = bson->key_buf.len; \
#define BASIC_CB_PREAMBLE \
const char *key; \
size_t len; \
bson_json_reader_bson_t *bson = &reader->bson; \
_bson_json_read_fixup_key (bson); \
key = bson->key; \
len = bson->key_buf.len; \
if (len > INT_MAX) { \
_bson_json_read_set_error (reader, "Failed to read JSON. key size %zu is too large. Max is %d", len, INT_MAX); \
return; \
} \
(void) 0
#define BASIC_CB_BAIL_IF_NOT_NORMAL(_type) \
if (bson->read_state != BSON_JSON_REGULAR) { \
Expand Down Expand Up @@ -624,7 +628,7 @@ _bson_json_read_integer (bson_json_reader_t *reader, uint64_t val, int64_t sign)
BASIC_CB_BAIL_IF_NOT_NORMAL ("integer");

if (val <= INT32_MAX || (sign == -1 && val <= (uint64_t) INT32_MAX + 1)) {
bson_append_int32 (STACK_BSON_CHILD, key, (int) len, (int) (val * sign));
bson_append_int32 (STACK_BSON_CHILD, key, (int) len, (int32_t) ((int64_t) val * sign));
} else if (sign == -1) {
#if defined(_WIN32) && !defined(__MINGW32__)
// Unary negation of unsigned integer is deliberate.
Expand Down
4 changes: 2 additions & 2 deletions src/libbson/src/jsonsl/jsonsl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1052,9 +1052,9 @@ void jsonsl_jpr_match_state_init(jsonsl_t jsn,
if (njprs == 0) {
return;
}
jsn->jprs = (jsonsl_jpr_t *)malloc(sizeof(jsonsl_jpr_t) * njprs);
jsn->jprs = (jsonsl_jpr_t *) bson_malloc (sizeof (jsonsl_jpr_t) * njprs);
jsn->jpr_count = njprs;
jsn->jpr_root = (size_t*)calloc(1, sizeof(size_t) * njprs * jsn->levels_max);
jsn->jpr_root = (size_t *) bson_malloc0 (sizeof (size_t) * njprs * jsn->levels_max);
memcpy(jsn->jprs, jprs, sizeof(jsonsl_jpr_t) * njprs);
/* Set the initial jump table values */

Expand Down
4 changes: 3 additions & 1 deletion src/libmongoc/src/mongoc/mongoc-client-session.c
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,9 @@ _max_time_ms_failure (bson_t *reply)
return true;
}

bson_iter_init (&iter, reply);
if (!bson_iter_init (&iter, reply)) {
return false;
}
if (bson_iter_find_descendant (&iter, "writeConcernError.codeName", &descendant) &&
BSON_ITER_HOLDS_UTF8 (&descendant) && 0 == strcmp (bson_iter_utf8 (&descendant, NULL), MAX_TIME_MS_EXPIRED)) {
return true;
Expand Down
8 changes: 6 additions & 2 deletions src/libmongoc/src/mongoc/mongoc-collection.c
Original file line number Diff line number Diff line change
Expand Up @@ -942,8 +942,12 @@ _mongoc_collection_index_keys_equal (const bson_t *expected, const bson_t *actua
bson_iter_t iter_expected;
bson_iter_t iter_actual;

bson_iter_init (&iter_expected, expected);
bson_iter_init (&iter_actual, actual);
if (!bson_iter_init (&iter_expected, expected)) {
return false;
}
if (!bson_iter_init (&iter_actual, actual)) {
return false;
}

while (bson_iter_next (&iter_expected)) {
/* If the key document has fewer items than expected, indexes are unequal
Expand Down
1 change: 1 addition & 0 deletions src/libmongoc/src/mongoc/mongoc-counters.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ mongoc_counters_calc_size (void)
if (mlib_cmp (size, >, pg_sz)) {
return size;
} else {
BSON_ASSERT (pg_sz > 0);
return (size_t) pg_sz;
}
#else
Expand Down
2 changes: 1 addition & 1 deletion src/libmongoc/src/mongoc/mongoc-server-description.c
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ mongoc_server_description_new_copy (const mongoc_server_description_t *descripti
const uint8_t *data = bson_get_data (&copy->last_hello_response) + offset; \
uint32_t len = description->FIELD.len; \
MONGOC_DEBUG_ASSERT (offset + len <= copy->last_hello_response.len); \
bson_init_static (&copy->FIELD, data, len); \
BSON_ASSERT (bson_init_static (&copy->FIELD, data, len)); \
} else { \
bson_init (&copy->FIELD); \
} \
Expand Down
1 change: 1 addition & 0 deletions src/libmongoc/src/mongoc/mongoc-socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,7 @@ _mongoc_socket_try_sendv_slow (mongoc_socket_t *sock, /* IN */
RETURN (ret ? ret : -1);
}

BSON_ASSERT (mlib_cmp (wrote, <=, SSIZE_MAX - ret));
ret += wrote;

if (mlib_cmp (wrote, !=, iov[i].iov_len)) {
Expand Down
2 changes: 2 additions & 0 deletions src/libmongoc/src/mongoc/mongoc-stream-tls-openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,8 @@ create_stream_with_ctx (
mongoc_stream_t *
mongoc_stream_tls_openssl_new (mongoc_stream_t *base_stream, const char *host, mongoc_ssl_opt_t *opt, int client)
{
BSON_ASSERT (opt);

SSL_CTX *ssl_ctx = _mongoc_openssl_ctx_new (opt);

if (!ssl_ctx) {
Expand Down
5 changes: 3 additions & 2 deletions src/libmongoc/src/mongoc/mongoc-stream-tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ mongoc_stream_tls_new_with_hostname (mongoc_stream_t *base_stream, const char *h

/* !client is only used for testing,
* when the streams are pretending to be the server */
if (!client || opt->weak_cert_validation) {
if (opt && (!client || opt->weak_cert_validation)) {
opt->allow_invalid_hostname = true;
}

#ifndef _WIN32
/* Silly check for Unix Domain Sockets */
if (!host || (host[0] == '/' && !access (host, F_OK))) {
if (opt && (!host || (host[0] == '/' && !access (host, F_OK)))) {
opt->allow_invalid_hostname = true;
}
#endif
Expand Down Expand Up @@ -209,6 +209,7 @@ mongoc_stream_tls_new_with_hostname_and_openssl_context (
mongoc_stream_t *base_stream, const char *host, mongoc_ssl_opt_t *opt, int client, SSL_CTX *ssl_ctx)
{
BSON_ASSERT (base_stream);
BSON_ASSERT (opt);

/* !client is only used for testing,
* when the streams are pretending to be the server */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,12 @@ _mongoc_topology_background_monitoring_stop (mongoc_topology_t *topology)
}

/* Signal all RTT monitors to shut down. */
bson_mutex_lock (&topology->tpld_modification_mtx);
for (size_t i = 0u; i < n_rtt_monitors; i++) {
server_monitor = mongoc_set_get_item (topology->rtt_monitors, i);
mongoc_server_monitor_request_shutdown (server_monitor);
}
bson_mutex_unlock (&topology->tpld_modification_mtx);
Comment on lines +314 to +319
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this mutex is relevant here. Do you have a link to the Coverity warning related to these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sent you the link in a DM.


for (size_t i = 0u; i < n_srv_monitors; i++) {
/* Wait for the thread to shutdown. */
Expand Down
10 changes: 8 additions & 2 deletions src/libmongoc/src/mongoc/mongoc-uri.c
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,10 @@ mongoc_uri_options_validate_names (const bson_t *a, const bson_t *b, bson_error_
/* Scan `a` looking for deprecated names
* where the canonical name was also used in `a`,
* or was used in `b`. */
bson_iter_init (&key_iter, a);
if (!bson_iter_init (&key_iter, a)) {
return false;
}

while (bson_iter_next (&key_iter)) {
key = bson_iter_key (&key_iter);
value = bson_iter_utf8_unsafe (&key_iter, &value_len);
Expand Down Expand Up @@ -1041,7 +1044,10 @@ mongoc_uri_apply_options (mongoc_uri_t *uri, const bson_t *options, bool from_dn
size_t value_len;
bool bval;

bson_iter_init (&iter, options);
if (!bson_iter_init (&iter, options)) {
return false;
}

while (bson_iter_next (&iter)) {
key = bson_iter_key (&iter);
canon = mongoc_uri_canonicalize_option (key);
Expand Down
2 changes: 2 additions & 0 deletions src/tools/mongoc-stat.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <sys/stat.h>
#include <unistd.h>

#include <mlib/cmp.h>

#pragma pack(1)
typedef struct {
Expand Down Expand Up @@ -109,6 +110,7 @@ mongoc_counters_new_from_pid (unsigned pid)
return NULL;
}

BSON_ASSERT (mlib_in_range (size_t, len));
size = len;

if (MAP_FAILED == (mem = mmap (NULL, size, PROT_READ, MAP_SHARED, fd, 0))) {
Expand Down