From bfb1f5fa9cea9175007c907f1d132b219f0a6c7f Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Tue, 1 Sep 2026 21:15:45 -0400 Subject: [PATCH] Introduce generic XDR freeing macros Add gp_xdr_proc, gp_xdr_free, and gp_xdr_free_ptr helper macros using C11 _Generic to automatically select the appropriate XDR procedure based on argument type. Replace repetitive xdr_free() and free() patterns across the codebase to reduce boilerplate and ensure safer memory cleanup. Assisted-by: Gemini:Gemini 3.7 Flash Signed-off-by: Simo Sorce --- src/client/gpm_accept_sec_context.c | 15 +++---------- src/client/gpm_common.c | 2 +- src/client/gpm_display_status.c | 3 +-- src/client/gpm_import_and_canon_name.c | 11 +++------ src/client/gpm_init_sec_context.c | 11 ++------- src/client/gpm_release_handle.c | 8 ++----- src/extract_ccache.c | 2 +- src/gp_common.h | 31 ++++++++++++++++++++++++++ src/gp_conv.c | 19 ++++++++-------- src/gp_creds.c | 2 +- src/gp_export.c | 14 +++++------- src/gp_rpc_indicate_mechs.c | 2 +- src/gp_rpc_process.c | 2 +- src/gp_util.c | 2 +- src/mechglue/gpp_acquire_cred.c | 5 +---- src/mechglue/gpp_creds.c | 7 ++---- src/mechglue/gpp_init_sec_context.c | 6 +---- src/mechglue/gss_plugin.c | 4 +--- tests/cli_srv_comm.c | 2 +- 19 files changed, 69 insertions(+), 79 deletions(-) diff --git a/src/client/gpm_accept_sec_context.c b/src/client/gpm_accept_sec_context.c index d50861532..deccf3a4e 100644 --- a/src/client/gpm_accept_sec_context.c +++ b/src/client/gpm_accept_sec_context.c @@ -134,14 +134,8 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status, gpm_free_xdrs(GSSX_ACCEPT_SEC_CONTEXT, &uarg, &ures); if (ret) { - if (ctx) { - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)ctx); - free(ctx); - } - if (name) { - xdr_free((xdrproc_t)xdr_gssx_name, (char *)name); - free(name); - } + gp_xdr_free_ptr(ctx); + gp_xdr_free_ptr(name); if (outbuf) { free(outbuf->value); free(outbuf); @@ -151,10 +145,7 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status, } /* always replace old ctx handle and set new */ - if (*context_handle) { - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)*context_handle); - free(*context_handle); - } + gp_xdr_free_ptr(*context_handle); *context_handle = ctx; return ret_maj; diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c index 928b9c187..9bdc87dbe 100644 --- a/src/client/gpm_common.c +++ b/src/client/gpm_common.c @@ -831,7 +831,7 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res) if (sockgrab) { gpm_release_sock(gpmctx); } - xdr_free((xdrproc_t)xdr_gp_rpc_msg, (char *)&msg); + gp_xdr_free(&msg); xdr_destroy(&xdr_call_ctx); if (recv_buffer != NULL) diff --git a/src/client/gpm_display_status.c b/src/client/gpm_display_status.c index e3aa4ea0a..b76b5aa30 100644 --- a/src/client/gpm_display_status.c +++ b/src/client/gpm_display_status.c @@ -8,8 +8,7 @@ static pthread_key_t gpm_last_status; static void gpm_destroy_last_status(void *arg) { gssx_status *status = (gssx_status *)arg; - xdr_free((xdrproc_t)xdr_gssx_status, (char *)status); - free(status); + gp_xdr_free_ptr(status); } void gpm_display_status_init_once(void) diff --git a/src/client/gpm_import_and_canon_name.c b/src/client/gpm_import_and_canon_name.c index 1805d4f74..ec39f7e43 100644 --- a/src/client/gpm_import_and_canon_name.c +++ b/src/client/gpm_import_and_canon_name.c @@ -92,10 +92,7 @@ OM_uint32 gpm_display_name(OM_uint32 *minor_status, ret_maj = GSS_S_COMPLETE; done: - if (output_name) { - xdr_free((xdrproc_t)xdr_gssx_name, (char *)output_name); - free(output_name); - } + gp_xdr_free_ptr(output_name); *minor_status = ret_min; return ret_maj; } @@ -406,10 +403,8 @@ OM_uint32 gpm_release_name(OM_uint32 *minor_status, { *minor_status = 0; - if (*input_name != NULL) { - xdr_free((xdrproc_t)xdr_gssx_name, (char *)(*input_name)); - free(*input_name); - *input_name = NULL; + if (input_name) { + gp_xdr_free_ptr(*input_name); } return GSS_S_COMPLETE; } diff --git a/src/client/gpm_init_sec_context.c b/src/client/gpm_init_sec_context.c index b84ff946f..8bf5e380f 100644 --- a/src/client/gpm_init_sec_context.c +++ b/src/client/gpm_init_sec_context.c @@ -162,11 +162,7 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status, *time_rec = ctx->lifetime; } } else { - if (ctx) { - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)ctx); - free(ctx); - ctx = NULL; - } + gp_xdr_free_ptr(ctx); if (outbuf) { free(outbuf->value); free(outbuf); @@ -174,10 +170,7 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status, } /* always replace old ctx handle and set new */ - if (*context_handle) { - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)*context_handle); - free(*context_handle); - } + gp_xdr_free_ptr(*context_handle); *context_handle = ctx; *minor_status = ret_min; diff --git a/src/client/gpm_release_handle.c b/src/client/gpm_release_handle.c index 2f707811a..cf2bd28ec 100644 --- a/src/client/gpm_release_handle.c +++ b/src/client/gpm_release_handle.c @@ -50,9 +50,7 @@ OM_uint32 gpm_release_cred(OM_uint32 *minor_status, memset(&arg->cred_handle.gssx_handle_u.cred_info, 0, sizeof(gssx_cred)); gpm_free_xdrs(GSSX_RELEASE_HANDLE, &uarg, &ures); done: - xdr_free((xdrproc_t)xdr_gssx_cred, (char *)r); - free(r); - *cred_handle = NULL; + gp_xdr_free_ptr(*cred_handle); return ret; } @@ -105,8 +103,6 @@ OM_uint32 gpm_delete_sec_context(OM_uint32 *minor_status, memset(&arg->cred_handle.gssx_handle_u.sec_ctx_info, 0, sizeof(gssx_cred)); gpm_free_xdrs(GSSX_RELEASE_HANDLE, &uarg, &ures); done: - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)r); - free(r); - *context_handle = NULL; + gp_xdr_free_ptr(*context_handle); return ret; } diff --git a/src/extract_ccache.c b/src/extract_ccache.c index b2425bab4..6f443b945 100644 --- a/src/extract_ccache.c +++ b/src/extract_ccache.c @@ -126,7 +126,7 @@ int extract_ccache(char *ccache_name, char *dest_ccache) if (ccache) krb5_cc_close(ctx, ccache); krb5_free_context(ctx); } - xdr_free((xdrproc_t)xdr_gssx_cred, (char *)&xcred); + gp_xdr_free(&xcred); gp_free_creds_handle(&handle); gss_release_cred(&ret_min, &gcred); free(data_out.data); diff --git a/src/gp_common.h b/src/gp_common.h index 8a53d6436..1f497e71f 100644 --- a/src/gp_common.h +++ b/src/gp_common.h @@ -66,6 +66,37 @@ ssize_t gp_safe_write(int fd, const void *buf, size_t count); char *gp_strerror(int errnum); #include "rpcgen/gss_proxy.h" +#include "rpcgen/gp_rpc.h" + +#define gp_xdr_proc(val) _Generic((val), \ + octet_string *: xdr_octet_string, \ + utf8string *: xdr_utf8string, \ + gssx_OID_set *: xdr_gssx_OID_set, \ + struct gssx_option *: xdr_gssx_option, \ + struct gssx_mech_attr *: xdr_gssx_mech_attr, \ + struct gssx_mech_info *: xdr_gssx_mech_info, \ + struct gssx_name_attr *: xdr_gssx_name_attr, \ + struct gssx_status *: xdr_gssx_status, \ + struct gssx_call_ctx *: xdr_gssx_call_ctx, \ + struct gssx_name *: xdr_gssx_name, \ + struct gssx_cred_element *: xdr_gssx_cred_element, \ + struct gssx_cred *: xdr_gssx_cred, \ + struct gssx_ctx *: xdr_gssx_ctx, \ + struct gssx_handle *: xdr_gssx_handle, \ + struct gssx_cb *: xdr_gssx_cb, \ + struct gp_rpc_msg *: xdr_gp_rpc_msg \ +) + +#define gp_xdr_free(val) do { \ + xdr_free((xdrproc_t)gp_xdr_proc(val), (char *)(val)); \ +} while(0) + +#define gp_xdr_free_ptr(ptr) do { \ + if (ptr) { \ + xdr_free((xdrproc_t)gp_xdr_proc(ptr), (char *)(ptr)); \ + safefree(ptr); \ + } \ +} while(0) union gp_rpc_arg { gssx_arg_release_handle release_handle; diff --git a/src/gp_conv.c b/src/gp_conv.c index b874b0648..82f3e3f16 100644 --- a/src/gp_conv.c +++ b/src/gp_conv.c @@ -228,9 +228,9 @@ int gp_conv_cb_to_gssx(gss_channel_bindings_t in, gssx_cb *out) done: if (ret) { - xdr_free((xdrproc_t)xdr_gssx_buffer, (char *)&out->initiator_address); - xdr_free((xdrproc_t)xdr_gssx_buffer, (char *)&out->acceptor_address); - xdr_free((xdrproc_t)xdr_gssx_buffer, (char *)&out->application_data); + gp_xdr_free(&out->initiator_address); + gp_xdr_free(&out->acceptor_address); + gp_xdr_free(&out->application_data); } return ret; } @@ -408,10 +408,10 @@ uint32_t gp_conv_name_to_gssx(uint32_t *min, gss_name_t in, gssx_name *_out) gss_release_buffer(&ret_min, &exported_name); gss_release_buffer(&ret_min, &exported_composite_name); if (ret_maj) { - xdr_free((xdrproc_t)xdr_gssx_buffer, (char *)&out.display_name); - xdr_free((xdrproc_t)xdr_gssx_OID, (char *)&out.name_type); - xdr_free((xdrproc_t)xdr_gssx_buffer, (char *)&out.exported_name); - xdr_free((xdrproc_t)xdr_gssx_buffer, (char *)&out.exported_composite_name); + gp_xdr_free(&out.display_name); + gp_xdr_free(&out.name_type); + gp_xdr_free(&out.exported_name); + gp_xdr_free(&out.exported_composite_name); } else { *_out = out; } @@ -590,8 +590,7 @@ int gp_copy_gssx_status_alloc(gssx_status *in, gssx_status **out) done: if (ret) { - xdr_free((xdrproc_t)xdr_gssx_status, (char *)o); - free(o); + gp_xdr_free_ptr(o); } return ret; } @@ -699,7 +698,7 @@ int gp_copy_gssx_name(gssx_name *in, gssx_name *out) done: if (ret) { - xdr_free((xdrproc_t)xdr_gssx_name, (char *)out); + gp_xdr_free(out); } return ret; } diff --git a/src/gp_creds.c b/src/gp_creds.c index 17855b8ec..f02cd5f3b 100644 --- a/src/gp_creds.c +++ b/src/gp_creds.c @@ -1222,7 +1222,7 @@ uint32_t gp_export_sync_creds(uint32_t *min, struct gp_call_ctx *gpcall, ret_maj = GSS_S_COMPLETE; done: - xdr_free((xdrproc_t)xdr_gssx_cred, (char *)&creds); + gp_xdr_free(&creds); *min = ret_min; return ret_maj; } diff --git a/src/gp_export.c b/src/gp_export.c index 075f9ac9f..8567006a8 100644 --- a/src/gp_export.c +++ b/src/gp_export.c @@ -772,7 +772,7 @@ uint32_t gp_export_ctx_id_to_gssx(uint32_t *min, int type, gss_OID mech, case EXP_CTX_PARTIAL: /* this happens only when a init_sec_context call returns a partially * initialized context so we return only what we have, not much */ - xdr_free((xdrproc_t)xdr_gssx_OID, (char *)&out->mech); + gp_xdr_free(&out->mech); ret = gp_conv_oid_to_gssx(mech, &out->mech); if (ret) { ret_maj = GSS_S_FAILURE; @@ -812,12 +812,10 @@ uint32_t gp_export_ctx_id_to_gssx(uint32_t *min, int type, gss_OID mech, } /* suppress names exported_composite_name, the kernel doesn't want * this information */ - xdr_free((xdrproc_t)xdr_gssx_buffer, - (char *)&out->src_name.exported_composite_name); + gp_xdr_free(&out->src_name.exported_composite_name); memset(&out->src_name.exported_composite_name, 0, sizeof(out->src_name.exported_composite_name)); - xdr_free((xdrproc_t)xdr_gssx_buffer, - (char *)&out->targ_name.exported_composite_name); + gp_xdr_free(&out->targ_name.exported_composite_name); memset(&out->targ_name.exported_composite_name, 0, sizeof(out->targ_name.exported_composite_name)); break; @@ -839,9 +837,9 @@ uint32_t gp_export_ctx_id_to_gssx(uint32_t *min, int type, gss_OID mech, gss_krb5_free_lucid_sec_context(&ret_min, lucid); } if (ret_maj) { - xdr_free((xdrproc_t)xdr_gssx_OID, (char *)&out->mech); - xdr_free((xdrproc_t)xdr_gssx_name, (char *)&out->src_name); - xdr_free((xdrproc_t)xdr_gssx_name, (char *)&out->targ_name); + gp_xdr_free(&out->mech); + gp_xdr_free(&out->src_name); + gp_xdr_free(&out->targ_name); } return ret_maj; } diff --git a/src/gp_rpc_indicate_mechs.c b/src/gp_rpc_indicate_mechs.c index de59e2744..c5d77d487 100644 --- a/src/gp_rpc_indicate_mechs.c +++ b/src/gp_rpc_indicate_mechs.c @@ -71,7 +71,7 @@ int gp_indicate_mechs(struct gp_call_ctx *gpcall UNUSED, /* temporarily skip any offender */ imr->mechs.mechs_len--; h--; - xdr_free((xdrproc_t)xdr_gssx_OID, (char *)&mi->mech); + gp_xdr_free(&mi->mech); continue; } diff --git a/src/gp_rpc_process.c b/src/gp_rpc_process.c index 1ac7c167f..496f17fa3 100644 --- a/src/gp_rpc_process.c +++ b/src/gp_rpc_process.c @@ -185,7 +185,7 @@ static int gp_rpc_decode_call_header(XDR *xdr_call_ctx, ret = 0; done: - xdr_free((xdrproc_t)xdr_gp_rpc_msg, (char *)&msg); + gp_xdr_free(&msg); return ret; } diff --git a/src/gp_util.c b/src/gp_util.c index cff7f1311..77ab013a2 100644 --- a/src/gp_util.c +++ b/src/gp_util.c @@ -212,7 +212,7 @@ uint32_t gp_add_option(gssx_option **options_val, u_int *options_len, done: if (ret) { - xdr_free((xdrproc_t)xdr_gssx_option, (char *)&opt); + gp_xdr_free(&opt); } return ret; } diff --git a/src/mechglue/gpp_acquire_cred.c b/src/mechglue/gpp_acquire_cred.c index 514fdd1e6..c3e437300 100644 --- a/src/mechglue/gpp_acquire_cred.c +++ b/src/mechglue/gpp_acquire_cred.c @@ -204,10 +204,7 @@ OM_uint32 gssi_acquire_cred_from(OM_uint32 *minor_status, maj = tmaj; min = tmin; } - if (in_cred_remote) { - xdr_free((xdrproc_t)xdr_gssx_cred, (char *)in_cred_remote); - free(in_cred_remote); - } + gp_xdr_free_ptr(in_cred_remote); if (maj == GSS_S_COMPLETE) { *output_cred_handle = (gss_cred_id_t)out_cred_handle; } else { diff --git a/src/mechglue/gpp_creds.c b/src/mechglue/gpp_creds.c index 677834db6..c84ffcf8e 100644 --- a/src/mechglue/gpp_creds.c +++ b/src/mechglue/gpp_creds.c @@ -65,10 +65,7 @@ uint32_t gpp_cred_handle_free(uint32_t *min, struct gpp_cred_handle *handle) maj = gss_release_cred(min, &handle->local); } - if (handle->remote) { - xdr_free((xdrproc_t)xdr_gssx_cred, (char *)handle->remote); - free(handle->remote); - } + gp_xdr_free_ptr(handle->remote); if (handle->store.count > 0) { for (size_t i = 0; i < handle->store.count; i++) { @@ -427,7 +424,7 @@ OM_uint32 gppint_get_def_creds(OM_uint32 *minor_status, } } - xdr_free((xdrproc_t)xdr_gssx_cred, (char *)&remote); + gp_xdr_free(&remote); if (maj == GSS_S_COMPLETE) { goto done; diff --git a/src/mechglue/gpp_init_sec_context.c b/src/mechglue/gpp_init_sec_context.c index f328993c0..123cebe8d 100644 --- a/src/mechglue/gpp_init_sec_context.c +++ b/src/mechglue/gpp_init_sec_context.c @@ -171,11 +171,7 @@ OM_uint32 gssi_init_sec_context(OM_uint32 *minor_status, &out_cred); if (maj == GSS_S_COMPLETE || maj == GSS_S_CONTINUE_NEEDED) { if (out_cred) { - if (cred_handle->remote) { - xdr_free((xdrproc_t)xdr_gssx_cred, - (char *)cred_handle->remote); - free(cred_handle->remote); - } + gp_xdr_free_ptr(cred_handle->remote); cred_handle->remote = out_cred; out_cred = NULL; /* failuire is not fatal */ diff --git a/src/mechglue/gss_plugin.c b/src/mechglue/gss_plugin.c index 4e2bc8fb7..9ef628c5f 100644 --- a/src/mechglue/gss_plugin.c +++ b/src/mechglue/gss_plugin.c @@ -524,9 +524,7 @@ uint32_t gpp_remote_to_local_ctx(uint32_t *minor, gssx_ctx **remote_ctx, free(mech.elements); (void)gss_release_buffer(&min, &wrap_token); - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)(*remote_ctx)); - free(*remote_ctx); - *remote_ctx = NULL; + gp_xdr_free_ptr(*remote_ctx); return maj; } diff --git a/tests/cli_srv_comm.c b/tests/cli_srv_comm.c index 413874312..05a6c97f6 100644 --- a/tests/cli_srv_comm.c +++ b/tests/cli_srv_comm.c @@ -94,7 +94,7 @@ int gp_send_accept_sec_context(int fd, return EFAULT; } - xdr_free((xdrproc_t)xdr_gp_rpc_msg, (char *)&msg); + gp_xdr_free(&msg); xdr_destroy(&xdr_call_ctx); xdr_destroy(&xdr_reply_ctx); return 0;