On 2/4/19, 11:15 AM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff"
<ovs-dev-boun...@openvswitch.org on behalf of b...@ovn.org> wrote:
On Sun, Feb 03, 2019 at 02:15:27PM -0800, Darrell Ball wrote:
> There are a few cases where padding may be undefined according to
> the C standard. Practically, it seems implementations don't have issue,
> but it is better to be safe. The code paths modified are not hot ones.
>
> Found by inspection.
>
> Signed-off-by: Darrell Ball <dlu...@gmail.com>
> ---
>
> v3: Removed an unnecessary change and limited scope of 2 others.
> v2: Found another one; will double check for others later.
>
> lib/conntrack.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index e1f4041..4c8d71b 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -748,6 +748,7 @@ conn_lookup(struct conntrack *ct, const struct
conn_key *key, long long now)
> {
> struct conn_lookup_ctx ctx;
> ctx.conn = NULL;
> + memset(&ctx.key, 0, sizeof ctx.key);
> ctx.key = *key;
> ctx.hash = conn_key_hash(key, ct->hash_basis);
> unsigned bucket = hash_to_bucket(ctx.hash);
We are talking about technical aspects of the C standard here. While
it's a somewhat petty distinction, padding bytes take unspecified
values, not undefined ones.
One can certainly assign values to the padding bytes with memset (or
otherwise using character types), but there's no guarantee (as far as I
know) that struct assignment doesn't overwrite the padding bytes with
unspecified values.
That is a very interesting interpretation of the standard.
My understanding is that there were only two possibilities
1/ Structure assignment copies the padding, which are always originally
initialized
2/ Structure assignment simply skips copying the padding, in which case the
destination padding bytes
are left as they were b4.
The third possibility of structure assignment overwriting the padding with
unspecified values is not
something I considered, because it seemed pointless to do that.
C11 6.2.6.1#11 essentially says that;
When a value is stored in an object of structure or union type,
including in a member object, the bytes of the object representation
that correspond to any padding bytes take unspecified values.51)
51) Thus, for example, structure assignment need not copy any
padding bits.
So I don't think there is value, standards-wise, to putting a memset
here. I don't know whether there is value, implementation-wise. Do you
know of a reason to believe it?
memcpy is certainly the first choice that came to mind
I chose memset because:
1/ It is highly optimized and combined with structure copy (also optimized) is
potentially faster than memcpy.
2/ memset zero and structure copy are easier to digest compared to memcpy,
although there are
two lines instead of one.
I am certainly fine either way though.
If we have a "delicate" type like this, then maybe we should use
memcpy() to copy it. (And be sure that we are somehow initializing
those padding bytes in the source of the memcpy().)
_______________________________________________
dev mailing list
d...@openvswitch.org
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=02%7C01%7Cdball%40vmware.com%7C8aea287cf6c24446a89e08d68ad51118%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636849045150275812&sdata=Zx0550Y62amgeZi7wDTnw144hBYPCLJgp1vRrzSdf%2BY%3D&reserved=0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
