[FFmpeg-devel] [PATCH] Remove useless preprocessor directives.

[Benoit Fouet]

On Thu, 17 Jun 2010 21:57:09 +0200 Reimar D?ffinger wrote:
> On Thu, Jun 17, 2010 at 03:32:02PM +0200, Benoit Fouet wrote:
> > yuv420_bgr32 and yuva420_bgr32 are only used if HAVE_7REGS is set.
> > The other solution would be to add an #else case.
> > 
> > This fixes the following warnings:
> > In file included from libswscale/x86/yuv2rgb_mmx.c:55:
> > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_rgb32_MMX?:
> > libswscale/x86/yuv2rgb_template.c:410: warning: no return statement in function returning non-void
> > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_bgr32_MMX?:
> > libswscale/x86/yuv2rgb_template.c:453: warning: no return statement in function returning non-void
> > In file included from libswscale/x86/yuv2rgb_mmx.c:62:
> > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_rgb32_MMX2?:
> > libswscale/x86/yuv2rgb_template.c:410: warning: no return statement in function returning non-void
> > libswscale/x86/yuv2rgb_template.c: In function ?yuva420_bgr32_MMX2?:
> > libswscale/x86/yuv2rgb_template.c:453: warning: no return statement in function returning non-void
> > ---
> >  x86/yuv2rgb_template.c |    4 ----
> >  1 files changed, 0 insertions(+), 4 deletions(-)
> > 
> > diff --git a/x86/yuv2rgb_template.c b/x86/yuv2rgb_template.c
> > index 5c062c1..d91c941 100644
> > --- a/x86/yuv2rgb_template.c
> > +++ b/x86/yuv2rgb_template.c
> > @@ -391,7 +391,6 @@ static inline int RENAME(yuva420_rgb32)(SwsContext *c, const uint8_t *src[],
> >                                          int srcSliceY, int srcSliceH,
> >                                          uint8_t *dst[], int dstStride[])
> >  {
> > -#if HAVE_7REGS
> >      int y, h_size;
> >  
> >      YUV2RGB_LOOP(4)
> > @@ -406,7 +405,6 @@ static inline int RENAME(yuva420_rgb32)(SwsContext *c, const uint8_t *src[],
> >      YUV2RGB_ENDLOOP(4)
> >      YUV2RGB_OPERANDS_ALPHA
> >      YUV2RGB_ENDFUNC
> > -#endif
> >  }
> >  
> >  static inline int RENAME(yuv420_bgr32)(SwsContext *c, const uint8_t *src[],
> > @@ -434,7 +432,6 @@ static inline int RENAME(yuva420_bgr32)(SwsContext *c, const uint8_t *src[],
> >                                          int srcSliceY, int srcSliceH,
> >                                          uint8_t *dst[], int dstStride[])
> >  {
> > -#if HAVE_7REGS
> >      int y, h_size;
> >  
> >      YUV2RGB_LOOP(4)
> > @@ -449,5 +446,4 @@ static inline int RENAME(yuva420_bgr32)(SwsContext *c, const uint8_t *src[],
> >      YUV2RGB_ENDLOOP(4)
> >      YUV2RGB_OPERANDS_ALPHA
> >      YUV2RGB_ENDFUNC
> > -#endif
> 
> This is complete nonsense, HAVE_7REGS etc. are used because the corresponding
> code can't _compile_, if it did compile it would work fine and we could use it.

Let's make things clear about nonsense: I had the warning without the
patch (meaning that 7REGS is not set on my machine); I don't have it
with the patch. So yes, of course, I did compile!
As I said, another solution would be:

diff --git a/x86/yuv2rgb_template.c b/x86/yuv2rgb_template.c
index 5c062c1..73e475c 100644
--- a/x86/yuv2rgb_template.c
+++ b/x86/yuv2rgb_template.c
@@ -406,6 +406,8 @@ static inline int RENAME(yuva420_rgb32)(SwsContext *c, const uint8_t *src[],
     YUV2RGB_ENDLOOP(4)
     YUV2RGB_OPERANDS_ALPHA
     YUV2RGB_ENDFUNC
+#else
+    return 0; /* Cannot happen */
 #endif
 }
 
@@ -449,5 +451,7 @@ static inline int RENAME(yuva420_bgr32)(SwsContext *c, const uint8_t *src[],
     YUV2RGB_ENDLOOP(4)
     YUV2RGB_OPERANDS_ALPHA
     YUV2RGB_ENDFUNC
+#else
+    return 0; /* Cannot happen */
 #endif
 }

And, of course, yet another solution would be to live with this warning.

> If you don't need/want the whole function, then put the whole function under
> the #if.

that wouldn't work either... as the code using it is in the form of:
if (HAVE_7REGS)
    foo = yuva420_rgb32_MMX;
You'll get an undeclared function error.

As a side note, those functions are marked as inline and we return the
function pointer in ff_yuv2rgb_init_mmx. I personaly find that weird.

Ben