Philippe Mathieu-Daudé <philmd@redhat.com> writes:
On 12/16/21 15:11, Alex Bennée wrote:
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
When experimenting raising GLIB_VERSION_MIN_REQUIRED to 2.68
(Fedora 34 provides GLib 2.68.1) we get:
hw/virtio/virtio-crypto.c:245:24: error: 'g_memdup' is deprecated: Use
'g_memdup2' instead [-Werror,-Wdeprecated-declarations]
...
g_memdup() has been updated by g_memdup2() to fix eventual security
issues (size argument is 32-bit and could be truncated / wrapping).
GLib recommends to copy their static inline version of g_memdup2():
https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538
Our glib-compat.h provides a comment explaining how to deal with
these deprecated declarations (see commit e71e8cc0355
"glib: enforce the minimum required version and warn about old APIs").
<snip>
+#define g_memdup2(m, s) g_memdup2_qemu(m, s)
+
As per our style wouldn't it make sense to just call it qemu_memdup(m,
s)?
I followed the documentation in include/glib-compat.h:
/*
* Note that because of the GLIB_VERSION_MAX_ALLOWED constant above,
allowing
* use of functions from newer GLib via this compat header needs a little
* trickery to prevent warnings being emitted.
*
* Consider a function from newer glib-X.Y that we want to use
*
* int g_foo(const char *wibble)
*
* We must define a static inline function with the same signature that does
* what we need, but with a "_qemu" suffix e.g.
*
* static inline void g_foo_qemu(const char *wibble)
* {
* #if GLIB_CHECK_VERSION(X, Y, 0)
* g_foo(wibble)
* #else
* g_something_equivalent_in_older_glib(wibble);
* #endif
* }
*
* The #pragma at the top of this file turns off -Wdeprecated-declarations,
* ensuring this wrapper function impl doesn't trigger the compiler warning
* about using too new glib APIs. Finally we can do
*
* #define g_foo(a) g_foo_qemu(a)
*
* So now the code elsewhere in QEMU, which *does* have the
* -Wdeprecated-declarations warning active, can call g_foo(...) as normal,
* without generating warnings.
*/
which is how g_unix_get_passwd_entry_qemu() is implemented.
Yet later we have qemu_g_test_slow following the style guide. Also I'm
confused by the usage of g_unix_get_passwd_entry_qemu because the only
place I see it in qga/commands-posix-ssh.c right before it does:
#define g_unix_get_passwd_entry_qemu(username, err) \
test_get_passwd_entry(username, err)
although I think that only hold when the file is built with
QGA_BUILD_UNIT_TEST.
Should we reword the documentation first?
The original wording in glib-compat.h was added by Daniel in 2018 but
the commit that added the password function comments:
Since the fallback version is still unsafe, I would rather keep the
_qemu postfix, to make sure it's not being misused by mistake. When/if
necessary, we can implement a safer fallback and drop the _qemu suffix.
So if we are going to make a distinction between a qemu prefix and
suffix we should agree that and add it to the style document.