[Ffmpeg-devel] [PATCH] MinGW and portability
Rich Felker
dalias
Tue Mar 28 23:39:33 CEST 2006
On Tue, Mar 28, 2006 at 09:43:54PM +0100, M?ns Rullg?rd wrote:
> > Simple patch needed for MinGW: replacing keyword near.
> >
> > Index: libavcodec/jpeg_ls.c
> > ===================================================================
> > RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/jpeg_ls.c,v
> > retrieving revision 1.5
> > diff -u -p -u -r1.5 jpeg_ls.c
> > --- libavcodec/jpeg_ls.c 2 Feb 2006 02:36:27 -0000 1.5
> > +++ libavcodec/jpeg_ls.c 28 Mar 2006 14:30:33 -0000
> > @@ -34,7 +34,7 @@ typedef struct JLSState{
> > int T1, T2, T3;
> > int A[367], B[367], C[365], N[367];
> > int limit, reset, bpp, qbpp, maxval, range;
> > - int near, twonear;
> > + int onenear, twonear;
>
> Rejected, "near" is not a keyword in any version of C. Fix the compiler.
This patch was already rejected once before. The solution is to add
the equivalent of -Dnear=msvc_sucks_near to your build options, not to
invasively alter valid C code to workaround MS bugs.
> > -extern int ffm_nopts;
> > +_DL_IMPORT extern int ffm_nopts;
>
> Ugh, yuck!! #define extern if you must. Changes like this are
> unacceptable.
Agree. Totally unacceptable.
Also, using the name _DL_IMPORT is illegal in a conforming C
application. This name is reserved for the implementation (__.* and
_[[:upper:]].* are reserved).
> > +#include "config.h"
> > #include <stdio.h>
> > #include <stdlib.h>
> > +#ifdef HAVE_INTTYPES_H
> > #include <inttypes.h>
> > +#endif
> > +#ifdef HAVE_STDINT_H
> > +#include <stdint.h>
> > +#endif
>
> Both inttypes.h and stdint.h are standard C. If you are missing them
> you should fix your environment.
Yes. This is easy to do..
> > #ifdef __MINGW32__
> > +#if __MINGW32_MAJOR_VERSION * 1000 + __MINGW32_MINOR_VERSION > 1002
>
> See above about ancient compilers.
Yes. This must be VERY ancient since most of the workarounds in these
patches are NOT needed in any version of mingw I've ever seen.
> > #define fseeko(x,y,z) fseeko64(x,y,z)
> > #define ftello(x) ftello64(x)
> > +#else
> > +#define fseeko(x,y,z) fseek(x,y,z)
> > +#define ftello(x) ftell(x)
> > +#endif
This is EXTREMELY WRONG and will break large file support on every
single platform except windows!!
> > +#if defined(WIN32) || defined(__MINGW32__)
> > +
> > +#include <stdarg.h>
> > +
> > +int snprintf(char *buf, const char *fmt, size_t size, ...) {
> > + va_list ap;
> > + int ret;
> > +
> > + va_start(ap, size);
> > + ret = vsprintf(buf, fmt, ap);
> > + va_end(ap);
> > + return ret;
> > +}
> > +
> > +#endif
>
> Potential security risk. Rejected.
More than just potential.
> Someone had put a "fix" like this in some code at work. A case turned
> up recently where it overflowed the buffer, which wasn't "large
> enough" after all, and it took quite some time to track down the
> reason for bizarre behavior.
Agree, this is completely not acceptable. Modern mingw has snprintf as
far as I know. If not then you need to write a complete snprintf
implementation... :)
It's also possible to emulate snprintf using a lot of hacks with
dynamic allocation and transformations on the format string to prevent
overflowing, but that's very difficult and risky if you make a
mistake..
> > -if (sdl-config --version) >/dev/null 2>&1 ; then
> > -if $cc -o $TMPE `sdl-config --cflags` $TMPC `sdl-config --libs` > /dev/null 2>&1 ; then
> > -_sdlversion=`sdl-config --version | sed 's/[^0-9]//g'`
> > +SDL_CONFIG="${cross_prefix}sdl-config"
> > +if ("${SDL_CONFIG}" --version) >/dev/null 2>&1 ; then
> > +if $cc -o $TMPE `"${SDL_CONFIG}" --cflags` $TMPC `"${SDL_CONFIG}" --libs` > /dev/null 2>&1 ; then
> > +_sdlversion=`"${SDL_CONFIG}" --version | sed 's/[^0-9]//g'`
> > if test "$_sdlversion" -lt 121 ; then
> > sdl_too_old=yes
> > else
> > @@ -1572,8 +1600,8 @@ if test "$pthreads" = "yes" ; then
> > fi
> > if test "$sdl" = "yes" ; then
> > echo "CONFIG_SDL=yes" >> config.mak
> > - echo "SDL_LIBS=`sdl-config --libs`" >> config.mak
> > - echo "SDL_CFLAGS=`sdl-config --cflags`" >> config.mak
> > + echo "SDL_LIBS=`"${SDL_CONFIG}" --libs`" >> config.mak
> > + echo "SDL_CFLAGS=`"${SDL_CONFIG}" --cflags`" >> config.mak
> > fi
> > if test "$texi2html" = "yes"; then
> > echo "BUILD_DOC=yes" >> config.mak
>
> This one looks sensible, assuming SDL is normally installed like that.
Yes, I don't see anything wrong with this..
Rich
More information about the ffmpeg-devel
mailing list