On 2016-08-01 09:15:30 +0300, Martin Storsjö wrote:
> This avoids issues with expanding the argument multiple times,
> and makes sure that it is of the right type for the following shifts.
>
> Even if the caller of a macro could be expected not to pass parameters
> that have side effects if expanded multiple times, these fallback
> codepaths are rarely, if ever, tested, so it is expected that such
> issues can arise.
>
> Thefore, for safety, make sure the fallback codepaths only expand
> the arguments once.
> ---
> With this in place, one could revert 25bacd0a0c and 014773b66b,
> and doesn't have to hunt for other issues in similar code not covered
> by fate.
>
> Or one could even make them proper inline functions, like in
> libavutil/arm/intreadwrite.h.
> ---
> libavutil/intreadwrite.h | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/libavutil/intreadwrite.h b/libavutil/intreadwrite.h
> index f77fd60..17b0b98 100644
> --- a/libavutil/intreadwrite.h
> +++ b/libavutil/intreadwrite.h
> @@ -210,7 +210,8 @@ union unaligned_16 { uint16_t l; }
> __attribute__((packed)) av_alias;
> ((const uint8_t*)(x))[1])
> #endif
> #ifndef AV_WB16
> -# define AV_WB16(p, d) do { \
> +# define AV_WB16(p, val) do { \
> + uint16_t d = val; \
> ((uint8_t*)(p))[1] = (d); \
> ((uint8_t*)(p))[0] = (d)>>8; \
> } while(0)
> @@ -222,7 +223,8 @@ union unaligned_16 { uint16_t l; }
> __attribute__((packed)) av_alias;
> ((const uint8_t*)(x))[0])
> #endif
> #ifndef AV_WL16
> -# define AV_WL16(p, d) do { \
> +# define AV_WL16(p, val) do { \
> + uint16_t d = val; \
> ((uint8_t*)(p))[0] = (d); \
> ((uint8_t*)(p))[1] = (d)>>8; \
> } while(0)
> @@ -236,7 +238,8 @@ union unaligned_16 { uint16_t l; }
> __attribute__((packed)) av_alias;
> ((const uint8_t*)(x))[3])
> #endif
> #ifndef AV_WB32
> -# define AV_WB32(p, d) do { \
> +# define AV_WB32(p, val) do { \
> + uint32_t d = val; \
> ((uint8_t*)(p))[3] = (d); \
> ((uint8_t*)(p))[2] = (d)>>8; \
> ((uint8_t*)(p))[1] = (d)>>16; \
> @@ -252,7 +255,8 @@ union unaligned_16 { uint16_t l; }
> __attribute__((packed)) av_alias;
> ((const uint8_t*)(x))[0])
> #endif
> #ifndef AV_WL32
> -# define AV_WL32(p, d) do { \
> +# define AV_WL32(p, val) do { \
> + uint32_t d = val; \
> ((uint8_t*)(p))[0] = (d); \
> ((uint8_t*)(p))[1] = (d)>>8; \
> ((uint8_t*)(p))[2] = (d)>>16; \
> @@ -272,7 +276,8 @@ union unaligned_16 { uint16_t l; }
> __attribute__((packed)) av_alias;
> (uint64_t)((const uint8_t*)(x))[7])
> #endif
> #ifndef AV_WB64
> -# define AV_WB64(p, d) do { \
> +# define AV_WB64(p, val) do { \
> + uint64_t d = val; \
> ((uint8_t*)(p))[7] = (d); \
> ((uint8_t*)(p))[6] = (d)>>8; \
> ((uint8_t*)(p))[5] = (d)>>16; \
> @@ -296,7 +301,8 @@ union unaligned_16 { uint16_t l; }
> __attribute__((packed)) av_alias;
> (uint64_t)((const uint8_t*)(x))[0])
> #endif
> #ifndef AV_WL64
> -# define AV_WL64(p, d) do { \
> +# define AV_WL64(p, val) do { \
> + uint64_t d = val; \
> ((uint8_t*)(p))[0] = (d); \
> ((uint8_t*)(p))[1] = (d)>>8; \
> ((uint8_t*)(p))[2] = (d)>>16; \
Ok and imo the preferable solution over the casts
Janne
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
