diff mbox series

[FFmpeg-devel] swscale/vscale: Increase type strictness

Message ID 20200426074957.12526-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 2fae0009942970f6331e2739bdcbd4b14a2485b0
Headers show
Series [FFmpeg-devel] swscale/vscale: Increase type strictness | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 26, 2020, 7:49 a.m. UTC 
libswscale/vscale.c makes extensive use of function pointers and in
doing so it converts these function pointers to and from a pointer to
void. Yet this is actually against the C standard:
C90 only guarantees that one can convert a pointer to any incomplete
type or object type to void* and back with the result comparing equal
to the original which makes pointers to void generic pointers to
incomplete or object type. Yet C90 lacks a generic function pointer
type.
C99 additionally guarantees that a pointer to a function of one type may
be converted to a pointer to a function of another type and when
converting back with the original and the result comparing equal.
This makes any function pointer type a generic function pointer type.
Yet even this does not make pointers to void generic function pointers.

Both GCC and Clang emit warnings for this when in pedantic mode.

This commit fixes this by using a union that can hold one member of any
of the required function pointer types to store the function pointer.
This works even for C90.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libswscale/vscale.c | 51 ++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

Comments

Michael Niedermayer April 27, 2020, 6:56 p.m. UTC | #1 
On Sun, Apr 26, 2020 at 09:49:57AM +0200, Andreas Rheinhardt wrote:
> libswscale/vscale.c makes extensive use of function pointers and in
> doing so it converts these function pointers to and from a pointer to
> void. Yet this is actually against the C standard:
> C90 only guarantees that one can convert a pointer to any incomplete
> type or object type to void* and back with the result comparing equal
> to the original which makes pointers to void generic pointers to
> incomplete or object type. Yet C90 lacks a generic function pointer
> type.
> C99 additionally guarantees that a pointer to a function of one type may
> be converted to a pointer to a function of another type and when
> converting back with the original and the result comparing equal.
> This makes any function pointer type a generic function pointer type.
> Yet even this does not make pointers to void generic function pointers.
> 
> Both GCC and Clang emit warnings for this when in pedantic mode.
> 
> This commit fixes this by using a union that can hold one member of any
> of the required function pointer types to store the function pointer.
> This works even for C90.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libswscale/vscale.c | 51 ++++++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)

patch ok

thx

[...]
Andreas Rheinhardt April 27, 2020, 10:20 p.m. UTC | #2 
Michael Niedermayer:
> On Sun, Apr 26, 2020 at 09:49:57AM +0200, Andreas Rheinhardt wrote:
>> libswscale/vscale.c makes extensive use of function pointers and in
>> doing so it converts these function pointers to and from a pointer to
>> void. Yet this is actually against the C standard:
>> C90 only guarantees that one can convert a pointer to any incomplete
>> type or object type to void* and back with the result comparing equal
>> to the original which makes pointers to void generic pointers to
>> incomplete or object type. Yet C90 lacks a generic function pointer
>> type.
>> C99 additionally guarantees that a pointer to a function of one type may
>> be converted to a pointer to a function of another type and when
>> converting back with the original and the result comparing equal.
>> This makes any function pointer type a generic function pointer type.
>> Yet even this does not make pointers to void generic function pointers.
>>
>> Both GCC and Clang emit warnings for this when in pedantic mode.
>>
>> This commit fixes this by using a union that can hold one member of any
>> of the required function pointer types to store the function pointer.
>> This works even for C90.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libswscale/vscale.c | 51 ++++++++++++++++++++++++++-------------------
>>  1 file changed, 29 insertions(+), 22 deletions(-)
> 
> patch ok
> 
> thx
> 
> [...]
> 
Applied, thanks.

- Andreas
diff mbox series

Patch

diff --git a/libswscale/vscale.c b/libswscale/vscale.c
index 72352dedb3..9ed227e908 100644
--- a/libswscale/vscale.c
+++ b/libswscale/vscale.c
@@ -25,7 +25,14 @@  typedef struct VScalerContext
     int32_t  *filter_pos;
     int filter_size;
     int isMMX;
-    void *pfn;
+    union {
+        yuv2planar1_fn      yuv2planar1;
+        yuv2planarX_fn      yuv2planarX;
+        yuv2interleavedX_fn yuv2interleavedX;
+        yuv2packed1_fn      yuv2packed1;
+        yuv2packed2_fn      yuv2packed2;
+        yuv2anyX_fn         yuv2anyX;
+    } pfn;
     yuv2packedX_fn yuv2packedX;
 } VScalerContext;
 
@@ -43,9 +50,9 @@  static int lum_planar_vscale(SwsContext *c, SwsFilterDescriptor *desc, int slice
     uint16_t *filter = inst->filter[0] + (inst->isMMX ? 0 : sliceY * inst->filter_size);
 
     if (inst->filter_size == 1)
-        ((yuv2planar1_fn)inst->pfn)((const int16_t*)src[0], dst[0], dstW, c->lumDither8, 0);
+        inst->pfn.yuv2planar1((const int16_t*)src[0], dst[0], dstW, c->lumDither8, 0);
     else
-        ((yuv2planarX_fn)inst->pfn)(filter, inst->filter_size, (const int16_t**)src, dst[0], dstW, c->lumDither8, 0);
+        inst->pfn.yuv2planarX(filter, inst->filter_size, (const int16_t**)src, dst[0], dstW, c->lumDither8, 0);
 
     if (desc->alpha) {
         int sp = first - desc->src->plane[3].sliceY;
@@ -55,9 +62,9 @@  static int lum_planar_vscale(SwsContext *c, SwsFilterDescriptor *desc, int slice
         uint16_t *filter = inst->filter[1] + (inst->isMMX ? 0 : sliceY * inst->filter_size);
 
         if (inst->filter_size == 1)
-            ((yuv2planar1_fn)inst->pfn)((const int16_t*)src[0], dst[0], dstW, c->lumDither8, 0);
+            inst->pfn.yuv2planar1((const int16_t*)src[0], dst[0], dstW, c->lumDither8, 0);
         else
-            ((yuv2planarX_fn)inst->pfn)(filter, inst->filter_size, (const int16_t**)src, dst[0], dstW, c->lumDither8, 0);
+            inst->pfn.yuv2planarX(filter, inst->filter_size, (const int16_t**)src, dst[0], dstW, c->lumDither8, 0);
     }
 
     return 1;
@@ -85,13 +92,13 @@  static int chr_planar_vscale(SwsContext *c, SwsFilterDescriptor *desc, int slice
         uint16_t *filter = inst->filter[0] + (inst->isMMX ? 0 : chrSliceY * inst->filter_size);
 
         if (c->yuv2nv12cX) {
-            ((yuv2interleavedX_fn)inst->pfn)(c, filter, inst->filter_size, (const int16_t**)src1, (const int16_t**)src2, dst1[0], dstW);
+            inst->pfn.yuv2interleavedX(c, filter, inst->filter_size, (const int16_t**)src1, (const int16_t**)src2, dst1[0], dstW);
         } else if (inst->filter_size == 1) {
-            ((yuv2planar1_fn)inst->pfn)((const int16_t*)src1[0], dst1[0], dstW, c->chrDither8, 0);
-            ((yuv2planar1_fn)inst->pfn)((const int16_t*)src2[0], dst2[0], dstW, c->chrDither8, 3);
+            inst->pfn.yuv2planar1((const int16_t*)src1[0], dst1[0], dstW, c->chrDither8, 0);
+            inst->pfn.yuv2planar1((const int16_t*)src2[0], dst2[0], dstW, c->chrDither8, 3);
         } else {
-            ((yuv2planarX_fn)inst->pfn)(filter, inst->filter_size, (const int16_t**)src1, dst1[0], dstW, c->chrDither8, 0);
-            ((yuv2planarX_fn)inst->pfn)(filter, inst->filter_size, (const int16_t**)src2, dst2[0], dstW, c->chrDither8, inst->isMMX ? (c->uv_offx2 >> 1) : 3);
+            inst->pfn.yuv2planarX(filter, inst->filter_size, (const int16_t**)src1, dst1[0], dstW, c->chrDither8, 0);
+            inst->pfn.yuv2planarX(filter, inst->filter_size, (const int16_t**)src2, dst2[0], dstW, c->chrDither8, inst->isMMX ? (c->uv_offx2 >> 1) : 3);
         }
     }
 
@@ -125,13 +132,13 @@  static int packed_vscale(SwsContext *c, SwsFilterDescriptor *desc, int sliceY, i
 
 
     if (c->yuv2packed1 && lum_fsize == 1 && chr_fsize == 1) { // unscaled RGB
-        ((yuv2packed1_fn)inst->pfn)(c, (const int16_t*)*src0, (const int16_t**)src1, (const int16_t**)src2,
+        inst->pfn.yuv2packed1(c, (const int16_t*)*src0, (const int16_t**)src1, (const int16_t**)src2,
                                     (const int16_t*)(desc->alpha ? *src3 : NULL),  *dst, dstW, 0, sliceY);
     } else if (c->yuv2packed1 && lum_fsize == 1 && chr_fsize == 2 &&
                chr_filter[2 * chrSliceY + 1] + chr_filter[2 * chrSliceY] == 4096 &&
                chr_filter[2 * chrSliceY + 1] <= 4096U) { // unscaled RGB
         int chrAlpha = chr_filter[2 * chrSliceY + 1];
-        ((yuv2packed1_fn)inst->pfn)(c, (const int16_t*)*src0, (const int16_t**)src1, (const int16_t**)src2,
+        inst->pfn.yuv2packed1(c, (const int16_t*)*src0, (const int16_t**)src1, (const int16_t**)src2,
                                     (const int16_t*)(desc->alpha ? *src3 : NULL),  *dst, dstW, chrAlpha, sliceY);
     } else if (c->yuv2packed2 && lum_fsize == 2 && chr_fsize == 2 &&
                lum_filter[2 * sliceY + 1] + lum_filter[2 * sliceY] == 4096 &&
@@ -145,7 +152,7 @@  static int packed_vscale(SwsContext *c, SwsFilterDescriptor *desc, int sliceY, i
         c->lumMmxFilter[3] = lum_filter[2 * sliceY]    * 0x10001;
         c->chrMmxFilter[2] =
         c->chrMmxFilter[3] = chr_filter[2 * chrSliceY] * 0x10001;
-        ((yuv2packed2_fn)inst->pfn)(c, (const int16_t**)src0, (const int16_t**)src1, (const int16_t**)src2, (const int16_t**)src3,
+        inst->pfn.yuv2packed2(c, (const int16_t**)src0, (const int16_t**)src1, (const int16_t**)src2, (const int16_t**)src3,
                     *dst, dstW, lumAlpha, chrAlpha, sliceY);
     } else { // general RGB
         if ((c->yuv2packed1 && lum_fsize == 1 && chr_fsize == 2) ||
@@ -195,7 +202,7 @@  static int any_vscale(SwsContext *c, SwsFilterDescriptor *desc, int sliceY, int
                         desc->alpha ? desc->dst->plane[3].line[dp3] : NULL };
 
     av_assert1(!c->yuv2packed1 && !c->yuv2packed2);
-    ((yuv2anyX_fn)inst->pfn)(c, lum_filter + sliceY * lum_fsize,
+    inst->pfn.yuv2anyX(c, lum_filter + sliceY * lum_fsize,
              (const int16_t**)src0, lum_fsize, chr_filter + sliceY * chr_fsize,
              (const int16_t**)src1, (const int16_t**)src2, chr_fsize, (const int16_t**)src3, dst, dstW, sliceY);
 
@@ -270,9 +277,9 @@  void ff_init_vscale_pfn(SwsContext *c,
             chrCtx->isMMX = use_mmx;
 
             --idx;
-            if (yuv2nv12cX)               chrCtx->pfn = yuv2nv12cX;
-            else if (c->vChrFilterSize == 1) chrCtx->pfn = yuv2plane1;
-            else                             chrCtx->pfn = yuv2planeX;
+            if (yuv2nv12cX)             chrCtx->pfn.yuv2interleavedX = yuv2nv12cX;
+            else if (c->vChrFilterSize == 1) chrCtx->pfn.yuv2planar1 = yuv2plane1;
+            else                             chrCtx->pfn.yuv2planarX = yuv2planeX;
         }
 
         lumCtx = c->desc[idx].instance;
@@ -283,8 +290,8 @@  void ff_init_vscale_pfn(SwsContext *c,
         lumCtx->filter_pos = c->vLumFilterPos;
         lumCtx->isMMX = use_mmx;
 
-        if (c->vLumFilterSize == 1) lumCtx->pfn = yuv2plane1;
-        else                        lumCtx->pfn = yuv2planeX;
+        if (c->vLumFilterSize == 1) lumCtx->pfn.yuv2planar1 = yuv2plane1;
+        else                        lumCtx->pfn.yuv2planarX = yuv2planeX;
 
     } else {
         lumCtx = c->desc[idx].instance;
@@ -303,12 +310,12 @@  void ff_init_vscale_pfn(SwsContext *c,
 
         if (yuv2packedX) {
             if (c->yuv2packed1 && c->vLumFilterSize == 1 && c->vChrFilterSize <= 2)
-                lumCtx->pfn = yuv2packed1;
+                lumCtx->pfn.yuv2packed1 = yuv2packed1;
             else if (c->yuv2packed2 && c->vLumFilterSize == 2 && c->vChrFilterSize == 2)
-                lumCtx->pfn = yuv2packed2;
+                lumCtx->pfn.yuv2packed2 = yuv2packed2;
             lumCtx->yuv2packedX = yuv2packedX;
         } else
-            lumCtx->pfn = yuv2anyX;
+            lumCtx->pfn.yuv2anyX = yuv2anyX;
     }
 }