On Fri, Mar 11, 2016 at 03:35:43PM +0100, Hendrik Leppkes wrote:
> On Wed, Mar 9, 2016 at 4:51 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
> > On Wed, Mar 9, 2016 at 4:35 PM, Michael Niedermayer
> > <mich...@niedermayer.cc> wrote:
> >> On Wed, Mar 09, 2016 at 04:27:12PM +0100, Hendrik Leppkes wrote:
> >>> On Wed, Mar 9, 2016 at 3:37 PM, Michael Niedermayer
> >>> <mich...@niedermayer.cc> wrote:
> >>> > document the issue with av_tempfile()
> >>> >
> >>> > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> >>> > ---
> >>> > libavcodec/libxvid.h | 2 --
> >>> > libavutil/file.c | 48 ++--------------------------------------
> >>> > libavutil/file.h | 1 +
> >>> > libavutil/file_open.c | 59
> >>> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >>> > libavutil/internal.h | 14 ++++++++++++
> >>> > 5 files changed, 76 insertions(+), 48 deletions(-)
> >>> >
> >>> > diff --git a/libavcodec/libxvid.h b/libavcodec/libxvid.h
> >>> > index bffe07d..ef9a5a9 100644
> >>> > --- a/libavcodec/libxvid.h
> >>> > +++ b/libavcodec/libxvid.h
> >>> > @@ -26,6 +26,4 @@
> >>> > * common functions for use with the Xvid wrappers
> >>> > */
> >>> >
> >>> > -int ff_tempfile(const char *prefix, char **filename);
> >>> > -
> >>> > #endif /* AVCODEC_LIBXVID_H */
> >>> > diff --git a/libavutil/file.c b/libavutil/file.c
> >>> > index 2a06be4..25381b1 100644
> >>> > --- a/libavutil/file.c
> >>> > +++ b/libavutil/file.c
> >>> > @@ -137,52 +137,8 @@ void av_file_unmap(uint8_t *bufptr, size_t size)
> >>> > #endif
> >>> > }
> >>> >
> >>> > -int av_tempfile(const char *prefix, char **filename, int log_offset,
> >>> > void *log_ctx)
> >>> > -{
> >>> > - FileLogContext file_log_ctx = { &file_log_ctx_class, log_offset,
> >>> > log_ctx };
> >>> > - int fd = -1;
> >>> > -#if !HAVE_MKSTEMP
> >>> > - void *ptr= tempnam(NULL, prefix);
> >>> > - if(!ptr)
> >>> > - ptr= tempnam(".", prefix);
> >>> > - *filename = av_strdup(ptr);
> >>> > -#undef free
> >>> > - free(ptr);
> >>> > -#else
> >>> > - size_t len = strlen(prefix) + 12; /* room for "/tmp/" and
> >>> > "XXXXXX\0" */
> >>> > - *filename = av_malloc(len);
> >>> > -#endif
> >>> > - /* -----common section-----*/
> >>> > - if (!*filename) {
> >>> > - av_log(&file_log_ctx, AV_LOG_ERROR, "ff_tempfile: Cannot
> >>> > allocate file name\n");
> >>> > - return AVERROR(ENOMEM);
> >>> > - }
> >>> > -#if !HAVE_MKSTEMP
> >>> > -# ifndef O_BINARY
> >>> > -# define O_BINARY 0
> >>> > -# endif
> >>> > -# ifndef O_EXCL
> >>> > -# define O_EXCL 0
> >>> > -# endif
> >>> > - fd = open(*filename, O_RDWR | O_BINARY | O_CREAT | O_EXCL, 0600);
> >>> > -#else
> >>> > - snprintf(*filename, len, "/tmp/%sXXXXXX", prefix);
> >>> > - fd = mkstemp(*filename);
> >>> > -#ifdef _WIN32
> >>> > - if (fd < 0) {
> >>> > - snprintf(*filename, len, "./%sXXXXXX", prefix);
> >>> > - fd = mkstemp(*filename);
> >>> > - }
> >>> > -#endif
> >>> > -#endif
> >>> > - /* -----common section-----*/
> >>> > - if (fd < 0) {
> >>> > - int err = AVERROR(errno);
> >>> > - av_log(&file_log_ctx, AV_LOG_ERROR, "ff_tempfile: Cannot open
> >>> > temporary file %s\n", *filename);
> >>> > - av_freep(filename);
> >>> > - return err;
> >>> > - }
> >>> > - return fd; /* success */
> >>> > +int av_tempfile(const char *prefix, char **filename, int log_offset,
> >>> > void *log_ctx) {
> >>> > + return avpriv_tempfile(prefix, filename, log_offset, log_ctx);
> >>> > }
> >>> >
> >>> > #ifdef TEST
> >>> > diff --git a/libavutil/file.h b/libavutil/file.h
> >>> > index e931be7..8666c7b 100644
> >>> > --- a/libavutil/file.h
> >>> > +++ b/libavutil/file.h
> >>> > @@ -62,6 +62,7 @@ void av_file_unmap(uint8_t *bufptr, size_t size);
> >>> > * @note On very old libcs it is necessary to set a secure umask before
> >>> > * calling this, av_tempfile() can't call umask itself as it is
> >>> > used in
> >>> > * libraries and could interfere with the calling application.
> >>> > + * @deprecated as fd numbers cannot be passed saftely between libs on
> >>> > some platforms
> >>> > */
> >>> > int av_tempfile(const char *prefix, char **filename, int log_offset,
> >>> > void *log_ctx);
> >>> >
> >>> > diff --git a/libavutil/file_open.c b/libavutil/file_open.c
> >>> > index 9e76127..6e58cc1 100644
> >>> > --- a/libavutil/file_open.c
> >>> > +++ b/libavutil/file_open.c
> >>> > @@ -92,6 +92,65 @@ int avpriv_open(const char *filename, int flags, ...)
> >>> > return fd;
> >>> > }
> >>> >
> >>> > +typedef struct FileLogContext {
> >>> > + const AVClass *class;
> >>> > + int log_offset;
> >>> > + void *log_ctx;
> >>> > +} FileLogContext;
> >>> > +
> >>> > +static const AVClass file_log_ctx_class = {
> >>> > + "TEMPFILE", av_default_item_name, NULL, LIBAVUTIL_VERSION_INT,
> >>> > + offsetof(FileLogContext, log_offset), offsetof(FileLogContext,
> >>> > log_ctx)
> >>> > +};
> >>> > +
> >>> > +int avpriv_tempfile(const char *prefix, char **filename, int
> >>> > log_offset, void *log_ctx)
> >>> > +{
> >>> > + FileLogContext file_log_ctx = { &file_log_ctx_class, log_offset,
> >>> > log_ctx };
> >>> > + int fd = -1;
> >>> > +#if !HAVE_MKSTEMP
> >>> > + void *ptr= tempnam(NULL, prefix);
> >>> > + if(!ptr)
> >>> > + ptr= tempnam(".", prefix);
> >>> > + *filename = av_strdup(ptr);
> >>> > +#undef free
> >>> > + free(ptr);
> >>> > +#else
> >>> > + size_t len = strlen(prefix) + 12; /* room for "/tmp/" and
> >>> > "XXXXXX\0" */
> >>> > + *filename = av_malloc(len);
> >>> > +#endif
> >>> > + /* -----common section-----*/
> >>> > + if (!*filename) {
> >>> > + av_log(&file_log_ctx, AV_LOG_ERROR, "ff_tempfile: Cannot
> >>> > allocate file name\n");
> >>> > + return AVERROR(ENOMEM);
> >>> > + }
> >>> > +#if !HAVE_MKSTEMP
> >>> > +# ifndef O_BINARY
> >>> > +# define O_BINARY 0
> >>> > +# endif
> >>> > +# ifndef O_EXCL
> >>> > +# define O_EXCL 0
> >>> > +# endif
> >>> > + fd = open(*filename, O_RDWR | O_BINARY | O_CREAT | O_EXCL, 0600);
> >>> > +#else
> >>> > + snprintf(*filename, len, "/tmp/%sXXXXXX", prefix);
> >>> > + fd = mkstemp(*filename);
> >>> > +#ifdef _WIN32
> >>> > + if (fd < 0) {
> >>> > + snprintf(*filename, len, "./%sXXXXXX", prefix);
> >>> > + fd = mkstemp(*filename);
> >>> > + }
> >>> > +#endif
> >>> > +#endif
> >>> > + /* -----common section-----*/
> >>> > + if (fd < 0) {
> >>> > + int err = AVERROR(errno);
> >>> > + av_log(&file_log_ctx, AV_LOG_ERROR, "ff_tempfile: Cannot open
> >>> > temporary file %s\n", *filename);
> >>> > + av_freep(filename);
> >>> > + return err;
> >>> > + }
> >>> > + return fd; /* success */
> >>> > +}
> >>> > +
> >>> > FILE *av_fopen_utf8(const char *path, const char *mode)
> >>> > {
> >>> > int fd;
> >>> > diff --git a/libavutil/internal.h b/libavutil/internal.h
> >>> > index c4bcf37..da76ca2 100644
> >>> > --- a/libavutil/internal.h
> >>> > +++ b/libavutil/internal.h
> >>> > @@ -244,6 +244,7 @@ void avpriv_request_sample(void *avc,
> >>> > #endif
> >>> >
> >>> > #define avpriv_open ff_open
> >>> > +#define avpriv_tempfile ff_tempfile
> >>> > #define PTRDIFF_SPECIFIER "Id"
> >>> > #define SIZE_SPECIFIER "Iu"
> >>> > #else
> >>> > @@ -319,6 +320,19 @@ static av_always_inline float ff_exp10f(float x)
> >>> > av_warn_unused_result
> >>> > int avpriv_open(const char *filename, int flags, ...);
> >>> >
> >>> > +/**
> >>> > + * Wrapper to work around the lack of mkstemp() on mingw.
> >>> > + * Also, tries to create file in /tmp first, if possible.
> >>> > + * *prefix can be a character constant; *filename will be allocated
> >>> > internally.
> >>> > + * @return file descriptor of opened file (or negative value
> >>> > corresponding to an
> >>> > + * AVERROR code on error)
> >>> > + * and opened file name in **filename.
> >>> > + * @note On very old libcs it is necessary to set a secure umask before
> >>> > + * calling this, av_tempfile() can't call umask itself as it is
> >>> > used in
> >>> > + * libraries and could interfere with the calling application.
> >>> > + */
> >>> > +int avpriv_tempfile(const char *prefix, char **filename, int
> >>> > log_offset, void *log_ctx);
> >>> > +
> >>> > int avpriv_set_systematic_pal2(uint32_t pal[256], enum AVPixelFormat
> >>> > pix_fmt);
> >>> >
> >>> > static av_always_inline av_const int avpriv_mirror(int x, int w)
> >>> > --
> >>> > 1.7.9.5
> >>>
> >>> This seems slightly confusing. You use avpriv but then define it to
> >>> ff? Why not just stick to ff everywhere? It should never be exported
> >>> from shared libraries to avoid this issue.
> >>> Maybe I'm missing something why this is done this way. :)
> >>
> >> the makefiles include file_open like this:
> >> OBJS-$(HAVE_LIBC_MSVCRT) += file_open.o
> >>
> >> so if we use file_open.c for *_tempfile it cant be ff_ if
> >> HAVE_LIBC_MSVCRT is not set as there would be none in the local lib
> >> i followed the same design as avpriv/ff_open()
> >> i would have favored ff_ as well if there was no pre-existing system
> >>
> >>
> >
> > I see, it only does this for msvcrt then.
> > I'll test the patch later.
> >
>
> Tested the set on the failing configuration, and it seems to work fine,
> thanks.patchset applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB During times of universal deceit, telling the truth becomes a revolutionary act. -- George Orwell
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
