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.
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
