From 986de6cfdc84468c3d1e106ee7af014015b11102 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 28 Oct 2019 11:44:04 -0500 Subject: [PATCH] rxgen: Introduce xdrfree_type() Currently, the most common way to free an object allocated by xdr is to call: xdr_free((xdrproc_t) xdr_foo, &foo); Which runs the given object through the xdr routines with the XDR_FREE operation. This works, but is not typesafe; if the wrong xdr_foo routine is given to xdr_free, we will silently run the wrong xdr routines, potentially freeing corrupt memory, etc. It is easy to make this mistake when dealing with many different XDR types, or dealing with various levels of indirection (e.g. an array of pointers to ...). It is also easy to make mistakes with strings; xdr_string() isn't really appropriate to give to xdr_free(), since xdr_string() takes 3 arguments, instead of the 2 arguments of most other xdr_type() routines. Commit bbb1e8adfe (xdr: Avoid xdr_string maxsize check when freeing) is an example of these issues. There is a typesafe way to free an xdr object, if the caller basically copies the implementation of xdr_free(): { XDR x; x.x_op = XDR_FREE; (void)xdr_foo(&x, &foo); } But this is rather cumbersome, and so is uncommon. To allow convenient freeing of xdr objects in a typesafe manner, introduce a generated function for each xdr type, called xdrfree_type(). This should result in the same behavior as xdr_free(), but does so in a typesafe manner and avoids the weird quirks with xdrproc_t and AFS_XDRPROC_NO_VARARG. Also define xdrfree_string(), to allow for convenient typesafe freeing of XDR strings directly. This commit does not add any calls to xdrfree_type(), but future commits will do so. Change-Id: Icf34e6de5d0da2b43b3e8ad3dc1acc74ef0e61dd Reviewed-on: https://gerrit.openafs.org/15497 Tested-by: BuildBot Reviewed-by: Michael Meffie --- src/rx/xdr.c | 9 +++++++++ src/rx/xdr_prototypes.h | 1 + src/rxgen/rpc_cout.c | 18 ++++++++++++++++++ src/rxgen/rpc_hout.c | 2 ++ 4 files changed, 30 insertions(+) diff --git a/src/rx/xdr.c b/src/rx/xdr.c index 88f7772c99..3353b72c33 100644 --- a/src/rx/xdr.c +++ b/src/rx/xdr.c @@ -577,6 +577,15 @@ xdr_wrapstring(XDR * xdrs, char **cpp) } #endif +void +xdrfree_string(char **cpp) +{ + XDR xdrs; + memset(&xdrs, 0, sizeof(xdrs)); + xdrs.x_op = XDR_FREE; + (void)xdr_string(&xdrs, cpp, MAX_AFS_UINT32); +} + void * xdr_alloc(afs_int32 size) { diff --git a/src/rx/xdr_prototypes.h b/src/rx/xdr_prototypes.h index cd0355021e..f3baedc3c8 100644 --- a/src/rx/xdr_prototypes.h +++ b/src/rx/xdr_prototypes.h @@ -57,6 +57,7 @@ extern bool_t xdr_bytes(XDR * xdrs, char **cpp, extern bool_t xdr_union(XDR * xdrs, enum_t * dscmp, caddr_t unp, struct xdr_discrim *choices, xdrproc_t dfault); extern bool_t xdr_string(XDR * xdrs, char **cpp, u_int maxsize); +extern void xdrfree_string(char **cpp); extern bool_t xdr_wrapstring(XDR * xdrs, char **cpp); extern void * xdr_alloc(afs_int32 size); extern void xdr_free(xdrproc_t proc, void *obj); diff --git a/src/rxgen/rpc_cout.c b/src/rxgen/rpc_cout.c index 70bbd677cc..8eced7bae7 100644 --- a/src/rxgen/rpc_cout.c +++ b/src/rxgen/rpc_cout.c @@ -115,10 +115,25 @@ undefined(char *type) } +static void +print_xdrfree(char *name) +{ + f_print(fout, "void\n"); + f_print(fout, "xdrfree_%s(%s *objp)", name, name); + f_print(fout, "{\n"); + f_print(fout, "\tXDR xdrs;\n"); + f_print(fout, "\tmemset(&xdrs, 0, sizeof(xdrs));\n"); + f_print(fout, "\txdrs.x_op = XDR_FREE;\n"); + f_print(fout, "\t(void)xdr_%s(&xdrs, objp);\n", name); + f_print(fout, "}\n"); +} + static void print_header(definition * def) { space(); + print_xdrfree(def->def_name); + f_print(fout, "bool_t\n"); f_print(fout, "xdr_%s(XDR *xdrs, ", def->def_name); f_print(fout, "%s ", def->def_name); @@ -462,6 +477,7 @@ print_hout(declaration * dec) f_print(fout, ";\n"); f_print(fout, "bool_t xdr_%s(XDR *xdrs, %s *objp);\n", dec->name, dec->name); + f_print(fout, "void xdrfree_%s(%s *objp);\n", dec->name, dec->name); } } @@ -477,6 +493,8 @@ print_cout(declaration * dec) print_ifstat(1, dec->prefix, dec->type, dec->rel, dec->array_max, "objp", dec->name); print_trailer(); + + print_xdrfree(dec->name); } } diff --git a/src/rxgen/rpc_hout.c b/src/rxgen/rpc_hout.c index 0acbbbe33c..251ddc92ed 100644 --- a/src/rxgen/rpc_hout.c +++ b/src/rxgen/rpc_hout.c @@ -92,6 +92,8 @@ print_datadef(definition * def) if (def->def_kind != DEF_CONST && (!IsRxgenDefinition(def))) { f_print(fout, "bool_t xdr_%s(XDR *xdrs, %s *objp);\n", def->def_name, def->def_name); + f_print(fout, "void xdrfree_%s(%s *objp);\n", def->def_name, + def->def_name); } if (def->def_kind != DEF_CONST && (!IsRxgenDefinition(def))) { f_print(fout, "\n");