Re: pkg/58717: wm/cde crashes in ttsession during startup on sparc64

["Martin Husemann via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR pkg/58717; it has been noted by GNATS.

From: Martin Husemann <martin%duskware.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: pkg/58717: wm/cde crashes in ttsession during startup on sparc64
Date: Sat, 5 Oct 2024 12:03:16 +0200

 This needs more explanation and should be filed as an upstream bug report
 and be resolved properly.
 
 You seem to patch mostly signed int vs unsigned int issues, which should not
 matter at this level. I would even silence any compiler warnings about
 mismatches via -Wno-.... options.
 
 Then there are a few xdr_long -> xdr_u_int translations which probably
 prevent the crashes you talked about, but it is hard to tell without
 more context if those are correct. If the long was an error and the
 value actually is int (or unsigned int) those patches would be OK, but
 if the real value is long you now only encode half of the data - either
 the lower or upper half, and it might break _LP64 little endian
 architectures.
 
 But the original code already casting the pointer is a red flag - you
 basically *never* should cast in this context, or something else is
 very wrong.
 
 This code is ugly, and the real way to fix it probably is to create 
 xdr_* helper functions to use for all the relevant types, like:
 
 static bool_t
 xdr_uid(XDR *xdrs, cost uid_t *uid)
 {
 	if (sizeof(*uid) == sizeof(long))
 		xdr_long(xdrs, uid);
 	else if (sizeof(*uid) == sizeof(int))
 		xdr_int(xdrs, uid);
 	else
 		assert(false);
 }
 
 Would be a nice use for C++ templates :-)
 You could hide this in macros, but that obfuscates it even more.
 
 Martin