On Sun, 19 Jun 2022, Soft Works wrote:
-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
Andreas Rheinhardt
Sent: Sunday, June 19, 2022 6:59 AM
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v17 1/5] libavutil: Add
wchartoutf8(), wchartoansi(), utf8toansi() and getenv_utf8()
Nil Admirari:
wchartoutf8() converts strings returned by WinAPI into UTF-8,
which is FFmpeg's preffered encoding.
Some external dependencies, such as AviSynth, are still
not Unicode-enabled. utf8toansi() converts UTF-8 strings
into ANSI in two steps: UTF-8 -> wchar_t -> ANSI.
wchartoansi() is responsible for the second step of the conversion.
Conversion in just one step is not supported by WinAPI.
Since these character converting functions allocate the buffer
of necessary size, they also facilitate the removal of MAX_PATH
limit
in places where fixed-size ANSI/WCHAR strings were used
as filename buffers.
getenv_utf8() wraps _wgetenv() converting its input from
and its output to UTF-8. Compared to plain getenv(),
getenv_utf8() requires a cleanup.
Because of that, in places that only test the existence of
an environment variable or compare its value with a string
consisting entirely of ASCII characters, the use of plain getenv()
is still preferred. (libavutil/log.c check_color_terminal()
is an example of such a place.)
Plain getenv() is also preffered in UNIX-only code,
such as bktr.c, fbdev_common.c, oss.c in libavdevice
or af_ladspa.c in libavfilter.
---
configure | 1 +
libavutil/getenv_utf8.h | 71
++++++++++++++++++++++++++++++++++++++
libavutil/wchar_filename.h | 51 +++++++++++++++++++++++++++
3 files changed, 123 insertions(+)
create mode 100644 libavutil/getenv_utf8.h
diff --git a/configure b/configure
index 3dca1c4bd3..fa37a74531 100755
--- a/configure
+++ b/configure
@@ -2272,6 +2272,7 @@ SYSTEM_FUNCS="
fcntl
getaddrinfo
getauxval
+ getenv
gethrtime
getopt
GetModuleHandle
diff --git a/libavutil/getenv_utf8.h b/libavutil/getenv_utf8.h
new file mode 100644
index 0000000000..161e3e6202
--- /dev/null
+++ b/libavutil/getenv_utf8.h
@@ -0,0 +1,71 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later
version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General
Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301 USA
+ */
+
+#ifndef AVUTIL_GETENV_UTF8_H
+#define AVUTIL_GETENV_UTF8_H
+
+#include <stdlib.h>
+
+#include "mem.h"
+
+#ifdef HAVE_GETENV
+
+#ifdef _WIN32
+
+#include "libavutil/wchar_filename.h"
+
+static inline char *getenv_utf8(const char *varname)
+{
+ wchar_t *varname_w, *var_w;
+ char *var;
+
+ if (utf8towchar(varname, &varname_w))
+ return NULL;
+ if (!varname_w)
+ return NULL;
+
+ var_w = _wgetenv(varname_w);
+ av_free(varname_w);
+
+ if (!var_w)
+ return NULL;
+ if (wchartoutf8(var_w, &var))
+ return NULL;
+
+ return var;
+
+ // No CP_ACP fallback compared to other *_utf8() functions:
+ // non UTF-8 strings must not be returned.
+}
+
+#else
+
+static inline char *getenv_utf8(const char *varname)
+{
+ return av_strdup(getenv(varname));
This forces allocations and frees in scenarios where this is wholly
unnecessary.
Why do you think this is unnecessary? At least on Windows, there is
no guarantee regarding the lifetime of strings returned from
getenv(). In case when some other code would call _putenv to set the
env variable, this can cause the previously returned string to become
invalid without the caller being able to know.
Yes, if you would keep the return value from getenv for too long, while
something else changes the environment in the same process, you'd have
such an issue. But that hasn't been a concern so far - right? And isn't
what we try to fix here.
This can be avoided by adding a custom deallocator for
strings returned via getenv_utf8: Namely a define/wrapper around
av_free in the _WIN32 and a no-op else.
I don't think I really understand what you mean, by the above 😉
He means that we could add a getenv_utf8_free(), which would be a noop on
unix, and keeping getenv_utf8() just returning plain getenv() on unix.
Then on Windows we'd do the extra alloc/free.
That sounds doable to me. There's a bigger risk of forgetting to free the
getenv_utf8 output (when tools like valgrind and asan wouldn't notice it
in unix), but that risk is probably acceptable.
// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
