[3/5] dxv: Don't use expressions with side effects in macro parameters

Message ID 1469706841-90972-3-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö July 28, 2016, 11:53 a.m.
AV_WL32 can be implemented as a macro that expands its parameters
multiple times (in case AV_HAVE_FAST_UNALIGNED isn't set and the
compiler doesn't support GCC attributes); make sure not to read
multiple times from the source in this case.
---
 libavcodec/dxv.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Diego Biurrun July 28, 2016, 6:58 p.m. | #1
On Thu, Jul 28, 2016 at 02:53:59PM +0300, Martin Storsjö wrote:
> AV_WL32 can be implemented as a macro that expands its parameters
> multiple times (in case AV_HAVE_FAST_UNALIGNED isn't set and the
> compiler doesn't support GCC attributes); make sure not to read
> multiple times from the source in this case.
> ---
>  libavcodec/dxv.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

OK

Diego
Diego Biurrun July 29, 2016, 3:37 p.m. | #2
On Thu, Jul 28, 2016 at 02:53:59PM +0300, Martin Storsjö wrote:
> AV_WL32 can be implemented as a macro that expands its parameters
> multiple times (in case AV_HAVE_FAST_UNALIGNED isn't set and the
> compiler doesn't support GCC attributes); make sure not to read
> multiple times from the source in this case.
> ---
>  libavcodec/dxv.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

This could be squashed into 2/5.

Diego
Martin Storsjö July 31, 2016, 7:29 p.m. | #3
On Fri, 29 Jul 2016, Diego Biurrun wrote:

> On Thu, Jul 28, 2016 at 02:53:59PM +0300, Martin Storsjö wrote:
>> AV_WL32 can be implemented as a macro that expands its parameters
>> multiple times (in case AV_HAVE_FAST_UNALIGNED isn't set and the
>> compiler doesn't support GCC attributes); make sure not to read
>> multiple times from the source in this case.
>> ---
>>  libavcodec/dxv.c | 18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> This could be squashed into 2/5.

Sure - done locally.

// Martin

Patch

diff --git a/libavcodec/dxv.c b/libavcodec/dxv.c
index 99327df..39b297a 100644
--- a/libavcodec/dxv.c
+++ b/libavcodec/dxv.c
@@ -121,8 +121,10 @@  static int dxv_decompress_dxt1(AVCodecContext *avctx)
     int pos = 2;
 
     /* Copy the first two elements */
-    AV_WL32(ctx->tex_data, bytestream2_get_le32(gbc));
-    AV_WL32(ctx->tex_data + 4, bytestream2_get_le32(gbc));
+    value = bytestream2_get_le32(gbc);
+    AV_WL32(ctx->tex_data, value);
+    value = bytestream2_get_le32(gbc);
+    AV_WL32(ctx->tex_data + 4, value);
 
     /* Process input until the whole texture has been filled */
     while (pos + 2 <= ctx->tex_size / 4) {
@@ -172,10 +174,14 @@  static int dxv_decompress_dxt5(AVCodecContext *avctx)
     int probe, check;
 
     /* Copy the first four elements */
-    AV_WL32(ctx->tex_data +  0, bytestream2_get_le32(gbc));
-    AV_WL32(ctx->tex_data +  4, bytestream2_get_le32(gbc));
-    AV_WL32(ctx->tex_data +  8, bytestream2_get_le32(gbc));
-    AV_WL32(ctx->tex_data + 12, bytestream2_get_le32(gbc));
+    value = bytestream2_get_le32(gbc);
+    AV_WL32(ctx->tex_data +  0, value);
+    value = bytestream2_get_le32(gbc);
+    AV_WL32(ctx->tex_data +  4, value);
+    value = bytestream2_get_le32(gbc);
+    AV_WL32(ctx->tex_data +  8, value);
+    value = bytestream2_get_le32(gbc);
+    AV_WL32(ctx->tex_data + 12, value);
 
     /* Process input until the whole texture has been filled */
     while (pos + 2 <= ctx->tex_size / 4) {