On Wed, 11 Apr 2012 at 10:08:20 +0200, Svante Signell wrote:
> severity 657793 important
Please do not exaggerate bug severities.
I can *just about* see how running a Quake dedicated server on the FreeBSD
kernel might be a rational thing to do, if you're really, really keen on
FreeBSD's firewalling... but I don't see any way in which the inability to
run a Quake dedicated server on Hurd could be construed to have
"a major effect on the usability of a package".
Running the client side of quakespasm on either kFreeBSD or Hurd makes
very little sense unless either can do better than VESA graphics.
On Sun, 29 Apr 2012 at 12:47:43 +0200, Robert Millan wrote:
> NMU to fix this has been uploaded to DELAYED/10. Debdiff is attached.
With that in mind, I don't think this is an appropriate use of an NMU.
Quakespasm has never been present on either of the affected architectures,
so there's certainly no regression here, and I doubt you can claim with
a straight face that any of our users have ever been disappointed to find
that they can't run their Quake server on non-Linux kernels.
Looking more closely at the code being patched, it's a workaround for
mis-design in Quakespasm, which makes it risky to apply patches to
fix the build without also testing thoroughly that the result actually works:
On Sat, 28 Jan 2012 at 21:31:53 +0000, Steven Chamberlain wrote:
> --- Quake/net_sys.h.orig 2012-01-28 21:28:41.000000000 +0000
> +++ Quake/net_sys.h 2012-01-28 21:28:46.000000000 +0000
> @@ -62,7 +62,7 @@
>
> #if defined(__FreeBSD__) || defined(__DragonFly__) || \
> defined(__OpenBSD__) || defined(__NetBSD__) || \
> - defined(__MACOSX__)
> + defined(__MACOSX__) || defined(__FreeBSD_kernel__)
> /* struct sockaddr has unsigned char sa_len as the first member in BSD
> * variants and the family member is also an unsigned char instead of an
> * unsigned short. This should matter only when PLATFORM_UNIX is defined,
I should have noticed that this was suspicious when I reviewed it before. Given
the structure of the Quakespasm code, it's a reasonable thing for porters
to do, but the right answer for high-quality code would be to make this list
of special cases irrelevant. Why would Quakespasm even care? Why would it be
relevant that Linux vs. BSD networking stacks have this difference?
It turns out that the reason this patch matters is that Quakespasm
defines "qsockaddr" to be a reinvention of struct sockaddr, then passes
a qsockaddr (with a cast, just to make sure there's no type-safety...) to
system functions that expect a struct sockaddr. It's not surprising that this
isn't portable.
Abstracting system differences can be useful if it actually abstracts things
(like GLib's GSocketAddress does), but this isn't abstraction, it's just
reinventing the system type, and then kludging around differences between
OSs by changing its redefinition to match theirs. The right way to be
portable would be to use the system's struct sockaddr as intended, perhaps by
replacing qsockaddr with something like this:
typedef union {
struct sockaddr sa;
struct sockaddr_in sa_in;
#ifdef HAVE_IPX
struct sockaddr_ipx sa_ipx;
#endif
/* ... and maybe IPv6, sockaddr_storage, etc. too */
} qsockaddr;
In the absence of a solution like that, we shouldn't blindly "fix the build"
without proper testing - if Quakespasm is making that sort of OS-specific
assumption, getting it to compile successfully on a particular kernel is
no guarantee that it'll work correctly, or fail without crashing, or for that
matter avoid having security vulnerabilities.
If what you're trying to achieve is to increase "proportion of Architecture:any
packages that compile" statistics, I wouldn't object to setting
Architecture: linux-any (or linux-any,kfreebsd-any with reasonable kFreeBSD
testing). I don't think it's to anyone's benefit to publish packages that
compile, have not been tested, and have a significant chance of not working.
S
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
