[FFmpeg-devel] [PATCH 1/2] avutil/file: Move av_tempfile() to avutil/file_open ff_tempfile()
Hendrik Leppkes
h.leppkes at gmail.com
Wed Mar 9 16:27:12 CET 2016
On Wed, Mar 9, 2016 at 3:37 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> document the issue with av_tempfile()
>
> Signed-off-by: Michael Niedermayer <michael at 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. :)
- Hendrik
More information about the ffmpeg-devel
mailing list