From: James Jones Date: Wed, 24 Aug 2022 22:01:46 +0000 (-0500) Subject: Silence coverity about fr_value_box_t assignment (CIDs below) (#4685) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=73e273752f8611417c4acc244aca0c7fc61e0aff;p=thirdparty%2Ffreeradius-server.git Silence coverity about fr_value_box_t assignment (CIDs below) (#4685) CIDs: #1508477, #1508480, #1508482, #1508484, #1508486 1508482 just uses an initializer. For the rest, we create an always-inlined function, fr_value_box_copy_unsafe(), to simply copy the raw bytes from one value box to another. Since it's a function, not a macro, the annotation should work. Since what fr_value_box_init() was doing didn't work, it too uses the function (and we had to move the box-to-box copy declarations and definitions so it would be visible in fr_value_box_init()). --- diff --git a/src/lib/redis/redis.c b/src/lib/redis/redis.c index 2ab8f90817b..a2f397f0e55 100644 --- a/src/lib/redis/redis.c +++ b/src/lib/redis/redis.c @@ -210,7 +210,7 @@ int fr_redis_reply_to_value_box(TALLOC_CTX *ctx, fr_value_box_t *out, redisReply fr_value_box_t *to_cast; if (dst_type != FR_TYPE_VOID) { - memset(&in, 0, sizeof(in)); + fr_value_box_copy_unsafe(&in, &(fr_value_box_t){}); to_cast = ∈ } else { to_cast = out; diff --git a/src/lib/server/snmp.c b/src/lib/server/snmp.c index 6f049931fa1..031674c6ef1 100644 --- a/src/lib/server/snmp.c +++ b/src/lib/server/snmp.c @@ -778,9 +778,7 @@ static ssize_t snmp_process_leaf(fr_dcursor_t *out, request_t *request, case FR_FREERADIUS_SNMP_OPERATION_VALUE_GET: { - fr_value_box_t data; - - memset(&data, 0, sizeof(data)); + fr_value_box_t data = (fr_value_box_t) {}; /* * Verify map is a leaf diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c index d18a5d1f37e..b71c45e9384 100644 --- a/src/lib/server/tmpl_eval.c +++ b/src/lib/server/tmpl_eval.c @@ -864,7 +864,7 @@ ssize_t _tmpl_to_atype(TALLOC_CTX *ctx, void *out, default: break; } - memcpy(&from_cast, to_cast, sizeof(from_cast)); + fr_value_box_copy_unsafe(&from_cast, to_cast); } RDEBUG4("Copying %zu bytes to %p from offset %zu", diff --git a/src/lib/util/value.h b/src/lib/util/value.h index 6ca56f7a363..e1c32f6e332 100644 --- a/src/lib/util/value.h +++ b/src/lib/util/value.h @@ -414,6 +414,38 @@ void fr_value_box_list_init(fr_value_box_list_t *list) } /** @} */ +/** @name Box to box copying + * + * @{ + */ +void fr_value_box_clear_value(fr_value_box_t *data) + CC_HINT(nonnull(1)); + +void fr_value_box_clear(fr_value_box_t *data) + CC_HINT(nonnull(1)); + +int fr_value_box_copy(TALLOC_CTX *ctx, fr_value_box_t *dst, const fr_value_box_t *src) + CC_HINT(nonnull(2,3)); + +void fr_value_box_copy_shallow(TALLOC_CTX *ctx, fr_value_box_t *dst, + const fr_value_box_t *src) + CC_HINT(nonnull(2,3)); + +/* + * fr_value_box_copy_unsafe() copies the raw bytes from one value box + * to another. Its uses are rare. + */ +static inline CC_HINT(nonnull, always_inline) +void fr_value_box_copy_unsafe(fr_value_box_t *dst, const fr_value_box_t *src) +{ + /* coverity[store_writes_const_field] */ + memcpy(dst, src, sizeof(*dst)); +} + +int fr_value_box_steal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_value_box_t *src) + CC_HINT(nonnull(2,3)); +/** @} */ + /** @name Value box assignment functions * * These functions allow C values to be assigned to value boxes. @@ -434,19 +466,11 @@ void fr_value_box_list_init(fr_value_box_list_t *list) static inline CC_HINT(nonnull(1), always_inline) void fr_value_box_init(fr_value_box_t *vb, fr_type_t type, fr_dict_attr_t const *enumv, bool tainted) { - /* - * An inventive way of defeating const on the type - * field so that we don't have the overhead of - * making these proper functions. - * - * Hopefully the compiler is smart enough to optimise - * this away. - */ - memcpy(vb, &(fr_value_box_t){ + fr_value_box_copy_unsafe(vb, &(fr_value_box_t){ .type = type, .enumv = enumv, .tainted = tainted - }, sizeof(*vb)); + }); fr_dlist_entry_init(&vb->entry); /* @@ -836,26 +860,7 @@ int fr_value_box_mark_safe(fr_value_box_t *box, uint16_t safe) void fr_value_box_mark_unsafe(fr_value_box_t *box) CC_HINT(nonnull); -/** @name Box to box copying - * - * @{ - */ -void fr_value_box_clear_value(fr_value_box_t *data) - CC_HINT(nonnull(1)); - -void fr_value_box_clear(fr_value_box_t *data) - CC_HINT(nonnull(1)); - -int fr_value_box_copy(TALLOC_CTX *ctx, fr_value_box_t *dst, const fr_value_box_t *src) - CC_HINT(nonnull(2,3)); - -void fr_value_box_copy_shallow(TALLOC_CTX *ctx, fr_value_box_t *dst, - const fr_value_box_t *src) - CC_HINT(nonnull(2,3)); -int fr_value_box_steal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_value_box_t *src) - CC_HINT(nonnull(2,3)); -/** @} */ /** @name Assign and manipulate binary-unsafe C strings *