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