Re: [PATCH] server: Avoid deprecated g_memdup
LGTM, thanks -- please commit.
On Thu, Sep 02, 2021 at 01:52:07PM -0500, Eric Blake wrote:
> glib now recommends that we use g_memdup2() to avoid accidental 32-bit
> truncation bugs on platforms where g_size is larger than guint:
>
> https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538
>
> and failure to do so causes noisy compilation due to deprecation
> warnings with glib 2.68:
>
> nbd-server.c: In function ‘parse_cfile’:
> nbd-server.c:1010:25: warning: ‘g_memdup’ is deprecated: Use 'g_memdup2' instead [-Wdeprecated-declarations]
> 1010 | SERVER *srv = serve_inc_ref(g_memdup(&s, sizeof(SERVER)));
> | ^~~~~~
> In file included from /usr/include/glib-2.0/glib.h:82,
> from nbd-server.c:117:
> /usr/include/glib-2.0/glib/gstrfuncs.h:257:23: note: declared here
> 257 | gpointer g_memdup (gconstpointer mem,
> | ^~~~~~~~
>
> Of course, we still want to build on platforms with older glib that
> lack g_memdup2(). Thankfully, it's easy enough to audit that all our
> current uses of g_memdup() do not overflow 32 bits.
> ---
> configure.ac | 5 +++++
> nbd-server.c | 7 ++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 48ba507..9504899 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -292,7 +292,9 @@ AC_CHECK_HEADERS([arpa/inet.h fcntl.h netdb.h netinet/in.h sys/ioctl.h sys/socke
> PKG_CHECK_MODULES(GLIB, [glib-2.0 >= 2.26.0 gthread-2.0 >= 2.26.0], [HAVE_GLIB=yes], AC_MSG_ERROR([Missing glib]))
>
> my_save_cflags="$CFLAGS"
> +my_save_libs="$LIBS"
> CFLAGS="-Wdeprecated-declarations -Werror $GLIB_CFLAGS"
> +LIBS="$GLIB_LIBS"
> AC_MSG_CHECKING([if we are using an old glib 2.0 library])
> AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
> [[#include <glib.h>]],
> @@ -306,7 +308,10 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
> AC_DEFINE(HAVE_OLD_GLIB, 0, [Define to 1 if you have an old glib library])
> ]
> )
> +dnl g_memdup2 added in glib-2.68
> +AC_CHECK_FUNCS([g_memdup2])
> CFLAGS="$my_save_cflags"
> +LIBS="$my_save_libs"
>
> AC_MSG_CHECKING([whether _BSD_SOURCE needs to be defined for DT_* macros])
> AC_PREPROC_IFELSE(
> diff --git a/nbd-server.c b/nbd-server.c
> index 0b32bcd..1eff99d 100644
> --- a/nbd-server.c
> +++ b/nbd-server.c
> @@ -154,6 +154,11 @@
> #include <gnutls/x509.h>
> #endif
>
> +#ifndef HAVE_G_MEMDUP2
> +/* Our uses of g_memdup2 below are safe from g_memdup's 32-bit overflow */
> +#define g_memdup2 g_memdup
> +#endif
> +
> /** Where our config file actually is */
> gchar* config_file_pos;
>
> @@ -1007,7 +1012,7 @@ GArray* parse_cfile(gchar* f, struct generic_conf *const genconf, bool expect_ge
> if(i>0 || !expect_generic) {
> s.servename = groups[i];
>
> - SERVER *srv = serve_inc_ref(g_memdup(&s, sizeof(SERVER)));
> + SERVER *srv = serve_inc_ref(g_memdup2(&s, sizeof(SERVER)));
> g_array_append_val(retval, srv);
> }
> #ifndef WITH_SDP
> --
> 2.31.1
>
>
--
w@uter.{be,co.za}
wouter@{grep.be,fosdem.org,debian.org}
Reply to: