Re: [PATCH 1/3] do full type check in COPY_ARRAY and MOVE_ARRAY

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 1, 2023, 03:03 UTC
Message-ID: <xmqq8rinhs7t.fsf@gitster.g>
In-Reply-To: <f3b9e9be-06ef-3773-a496-da5e479f9d49@web.de>

René Scharfe <l.s.r@web.de> writes:

> Extend the element size comparison between source and destination with
> a full type check using an assignment.  It is not actually evaluated and
> even optimized out, but compilers check the types before getting to that
> point, and report mismatches.

It is not actually evaluated and is (even) optimized out, which is a neat and clever trick. The above made me read it twice, though, as I misread on my first read that you are saying that the code is left in the binary but in an unreachable form (i.e. "it is not (evaluated or optimized out"), though.

Show 5 quoted lines

> The stricter check improves safety, as it catches attempts to copy
> between different types that happen to have the same size.  The size
> check is still needed to avoid allowing copies from a array with a
> smaller element type to a bigger one, e.g. from a char array to an int
> array, which would be allowed by the assignment check alone.

Do you mean

	int *ip;
	char *cp;

	(0 ? (*(ip) = *(cp), 0) : 0); /* silently allowed */
	(0 ? (*(cp) = *(ip), 0) : 0); /* truncation noticed */

Presumably we cannot catch
	
	int *ip;
	unsinged *up;

	(0 ? (*(ip) = *(up), 0) : 0);
	(0 ? (*(up) = *(ip), 0) : 0);

with the approach? I think what you ideally want to enforce is that typeof(dst) is exactly typeof(src), or typeof(src) sans constness (i.e. you can populate non-const array from a const template)?

Show 15 quoted lines

>  #define COPY_ARRAY(dst, src, n) copy_array((dst), (src), (n), sizeof(*(dst)) + \
> +	(0 ? (*(dst) = *(src), 0) : 0) + \
>  	BUILD_ASSERT_OR_ZERO(sizeof(*(dst)) == sizeof(*(src))))
>  static inline void copy_array(void *dst, const void *src, size_t n, size_t size)
>  {
> @@ -1102,6 +1103,7 @@ static inline void copy_array(void *dst, const void *src, size_t n, size_t size)
>  }
>
>  #define MOVE_ARRAY(dst, src, n) move_array((dst), (src), (n), sizeof(*(dst)) + \
> +	(0 ? (*(dst) = *(src), 0) : 0) + \
>  	BUILD_ASSERT_OR_ZERO(sizeof(*(dst)) == sizeof(*(src))))
>  static inline void move_array(void *dst, const void *src, size_t n, size_t size)
>  {
> --
> 2.39.0