[FFmpeg-devel] [PATCH 1/2] avutil/file: Move av_tempfile() to avutil/file_open ff_tempfile()
Michael Niedermayer
michael at niedermayer.cc
Wed Mar 9 16:35:38 CET 2016
On Wed, Mar 09, 2016 at 04:27:12PM +0100, Hendrik Leppkes wrote:
> 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. :)
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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160309/e5c132bf/attachment.sig>
More information about the ffmpeg-devel
mailing list