Skip to content
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

DAOS-15744 ddb: Fixes for Coverity Issues #14275

Merged
merged 5 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion ci/jira_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
# valid. We've never checked/enforced these before so there have been a lot of values used in the
# past.
VALID_COMPONENTS = ('agent', 'build', 'ci', 'csum', 'doc', 'gha', 'il', 'md', 'mercury',
'packaging', 'pil4dfs', 'swim', 'test', 'tools')
'packaging', 'pil4dfs', 'swim', 'test', 'tools', 'ddb')

# Expected ticket prefix.
VALID_TICKET_PREFIX = ('DAOS', 'CORCI', 'SRE')
Expand Down
10 changes: 6 additions & 4 deletions src/utils/ddb/ddb.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2022-2023 Intel Corporation.
* (C) Copyright 2022-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -772,12 +772,14 @@ ddb_run_cmd(struct ddb_ctx *ctx, const char *cmd_str, bool write_mode)
cmd_copy[strlen(cmd_copy) - 1] = '\0';

rc = ddb_str2argv_create(cmd_copy, &parse_args);
if (!SUCCESS(rc))
D_GOTO(done, rc);
if (!SUCCESS(rc)) {
D_FREE(cmd_copy);
return rc;
}

if (parse_args.ap_argc == 0) {
D_ERROR("Nothing parsed\n");
return -DER_INVAL;
D_GOTO(done, rc = -DER_INVAL);
}

rc = ddb_parse_cmd_args(ctx, parse_args.ap_argc, parse_args.ap_argv, &info);
Expand Down
22 changes: 16 additions & 6 deletions src/utils/ddb/ddb_parse.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2019-2022 Intel Corporation.
* (C) Copyright 2019-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand All @@ -10,6 +10,14 @@
#include "ddb_common.h"
#include "ddb_parse.h"

void
safe_strcat(char *dst, const char *src, size_t dst_size)
{
size_t remaining_space = dst_size - strlen(dst) - 1; // Subtract 1 for null terminator

strncat(dst, src, remaining_space);
}

int
vos_path_parse(const char *path, struct vos_file_parts *vos_file_parts)
{
Expand All @@ -29,8 +37,8 @@ vos_path_parse(const char *path, struct vos_file_parts *vos_file_parts)
while (tok != NULL && rc != 0) {
rc = uuid_parse(tok, vos_file_parts->vf_pool_uuid);
if (!SUCCESS(rc)) {
strcat(vos_file_parts->vf_db_path, "/");
strcat(vos_file_parts->vf_db_path, tok);
safe_strcat(vos_file_parts->vf_db_path, "/", DB_PATH_LEN);
safe_strcat(vos_file_parts->vf_db_path, tok, DB_PATH_LEN);
}
tok = strtok(NULL, "/");
}
Expand Down Expand Up @@ -83,14 +91,16 @@ ddb_str2argv_create(const char *buf, struct argv_parsed *parse_args)
parse_args->ap_argv = we->we_wordv;
parse_args->ap_ctx = we;

return rc;
return 0;
}

void
ddb_str2argv_free(struct argv_parsed *parse_args)
{
wordfree(parse_args->ap_ctx);
D_FREE(parse_args->ap_ctx);
if (parse_args->ap_ctx != NULL) {
wordfree(parse_args->ap_ctx);
D_FREE(parse_args->ap_ctx);
}
}

int
Expand Down
6 changes: 3 additions & 3 deletions src/utils/ddb/ddb_parse.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2019-2022 Intel Corporation.
* (C) Copyright 2019-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand All @@ -21,9 +21,9 @@ struct program_args {
bool pa_write_mode;
bool pa_get_help;
};

#define DB_PATH_LEN 64
struct vos_file_parts {
char vf_db_path[64];
char vf_db_path[DB_PATH_LEN];
uuid_t vf_pool_uuid;
char vf_vos_file[16];
uint32_t vf_target_idx;
Expand Down
10 changes: 9 additions & 1 deletion src/utils/ddb/ddb_tree_path.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2023 Intel Corporation.
* (C) Copyright 2023-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -187,6 +187,8 @@ parse_key(const char *key_str, struct dv_indexed_tree_path *itp, enum path_parts
if (strlen(key_str) == 0)
return 0;

D_ASSERT(key_part < PATH_PART_END);

/* is an index */
if (key_str[0] == '[') {
uint32_t idx;
Expand Down Expand Up @@ -512,19 +514,22 @@ itp_unset_cont(struct dv_indexed_tree_path *itp)
int
itp_idx(struct dv_indexed_tree_path *itp, enum path_parts part_key)
{
D_ASSERT(part_key < PATH_PART_END);
return itp->itp_parts[part_key].itp_part_idx;
}

bool
itp_has_complete(struct dv_indexed_tree_path *itp, enum path_parts part_key)
{
D_ASSERT(part_key < PATH_PART_END);
return itp->itp_parts[part_key].itp_has_part_value &&
itp->itp_parts[part_key].itp_has_part_idx;
}

bool
itp_has(struct dv_indexed_tree_path *itp, enum path_parts part_key)
{
D_ASSERT(part_key < PATH_PART_END);
return itp->itp_parts[part_key].itp_has_part_value ||
itp->itp_parts[part_key].itp_has_part_idx;
}
Expand All @@ -539,12 +544,14 @@ itp_has_value(struct dv_indexed_tree_path *itp)
bool
itp_has_idx(struct dv_indexed_tree_path *itp, enum path_parts part_key)
{
D_ASSERT(part_key < PATH_PART_END);
return itp->itp_parts[part_key].itp_has_part_idx;
}

bool
itp_has_part_value(struct dv_indexed_tree_path *itp, enum path_parts part_key)
{
D_ASSERT(part_key < PATH_PART_END);
return itp->itp_parts[part_key].itp_has_part_value;
}

Expand Down Expand Up @@ -633,6 +640,7 @@ itp_verify(struct dv_indexed_tree_path *itp)
static union itp_part_type *
itp_value(struct dv_indexed_tree_path *itp, enum path_parts path_key)
{
D_ASSERT(path_key < PATH_PART_END);
return &itp->itp_parts[path_key].itp_part_value;
}

Expand Down
12 changes: 5 additions & 7 deletions src/utils/ddb/ddb_vos.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2022-2023 Intel Corporation.
* (C) Copyright 2022-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -379,7 +379,7 @@ vos_iterator_type_to_path_part(vos_iter_type_t type)
static enum path_parts
vos_enum_to_path_part(vos_iter_type_t t)
{
enum path_parts map[VOS_ITER_LARGEST];
enum path_parts map[VOS_ITER_LARGEST] = {0};

map[VOS_ITER_OBJ] = PATH_PART_OBJ;
map[VOS_ITER_DKEY] = PATH_PART_DKEY;
Expand All @@ -393,7 +393,7 @@ vos_enum_to_path_part(vos_iter_type_t t)
static enum path_parts
vos_enum_to_parent_path_part(vos_iter_type_t t)
{
int map[VOS_ITER_LARGEST];
int map[VOS_ITER_LARGEST] = {0};

map[VOS_ITER_OBJ] = PATH_PART_CONT;
map[VOS_ITER_DKEY] = PATH_PART_OBJ;
Expand Down Expand Up @@ -1741,11 +1741,9 @@ sync_cb(struct ddbs_sync_info *info, void *cb_args)

/* Try to delete the target first */
rc = smd_pool_del_tgt(pool_id, info->dsi_hdr->bbh_vos_id, st);
if (!SUCCESS(rc)) {
if (!SUCCESS(rc))
/* Ignore error for now ... might not exist*/
D_WARN("delete target failed: "DF_RC"\n", DP_RC(rc));
rc = 0;
}
D_WARN("delete target failed: " DF_RC "\n", DP_RC(rc));

rc = smd_pool_add_tgt(pool_id, info->dsi_hdr->bbh_vos_id,
info->dsi_hdr->bbh_blob_id, st, blob_size);
Expand Down
7 changes: 5 additions & 2 deletions src/utils/ddb/tests/ddb_commands_tests.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2022-2023 Intel Corporation.
* (C) Copyright 2022-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -389,8 +389,11 @@ dcv_suit_teardown(void **state)
{
struct dt_vos_pool_ctx *tctx = *state;

if (tctx == NULL)
if (tctx == NULL) {
fail_msg("Test not setup correctly");
return -DER_UNKNOWN;
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this is just test code, but DER_UNKNOWN is not friendly for debugging or triaging purposes. please consider changing that in a follow on

}

assert_success(dv_pool_close(tctx->dvt_poh));
ddb_teardown_vos(state);

Expand Down
7 changes: 5 additions & 2 deletions src/utils/ddb/tests/ddb_main_tests.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2022 Intel Corporation.
* (C) Copyright 2022-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -251,8 +251,11 @@ ddb_main_suit_teardown(void **state)
{
struct dt_vos_pool_ctx *tctx = *state;

if (tctx == NULL)
if (tctx == NULL) {
fail_msg("Test not setup correctly");
return -DER_UNKNOWN;
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

}

assert_success(dv_pool_close(tctx->dvt_poh));
ddb_teardown_vos(state);

Expand Down
3 changes: 1 addition & 2 deletions src/utils/ddb/tests/ddb_parse_tests.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2022-2023 Intel Corporation.
* (C) Copyright 2022-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -40,7 +40,6 @@ assert_parsed_fail(const char *str)
int rc;

rc = ddb_str2argv_create(str, &parse_args);
ddb_str2argv_free(&parse_args);
ryon-jensen marked this conversation as resolved.
Show resolved Hide resolved
assert_rc_equal(-DER_INVAL, rc);
}

Expand Down
21 changes: 13 additions & 8 deletions src/utils/ddb/tests/ddb_test_driver.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2022 Intel Corporation.
* (C) Copyright 2022-2024 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand Down Expand Up @@ -208,20 +208,20 @@ int
ddb_test_pool_setup(struct dt_vos_pool_ctx *tctx)
{
int rc;
uint64_t size = (1ULL << 30);
struct stat st = {0};
uint64_t size = (1ULL << 30);
char *pool_uuid = "12345678-1234-1234-1234-123456789012";
int mkdir_result;

if (strlen(tctx->dvt_pmem_file) == 0) {
char dir[64] = {0};

sprintf(dir, "/mnt/daos/%s", pool_uuid);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also use snprintf here

if (stat(dir, &st) == -1) {
if (!SUCCESS(mkdir(dir, 0700))) {
rc = daos_errno2der(errno);
return rc;
}
mkdir_result = mkdir(dir, 0700);
if (mkdir_result == -1 && errno != EEXIST) {
rc = daos_errno2der(errno);
return rc;
}

snprintf(tctx->dvt_pmem_file, ARRAY_SIZE(tctx->dvt_pmem_file),
"%s/ddb_vos_test", dir);
}
Expand Down Expand Up @@ -314,6 +314,11 @@ ddb_teardown_vos(void **state)
{
struct dt_vos_pool_ctx *tctx = *state;

if (tctx == NULL) {
fail_msg("Test context not setup correctly");
return -DER_UNKNOWN;
}

vos_self_init("/mnt/daos", false, 0);
assert_success(vos_pool_destroy(tctx->dvt_pmem_file, tctx->dvt_pool_uuid));
vos_self_fini();
Expand Down
Loading