Re: Confusing definitions and declarations of mig_dealloc_reply_port()

[Svante Signell]

On Wed, 2015-11-04 at 22:34 -0300, Diego Nieto Cid wrote:
> 
> 2015-11-04 16:03 GMT-03:00 Svante Signell <svante.signell@gmail.com>:
> > On Wed, 2015-11-04 at 18:57 +0100, Samuel Thibault wrote:
> > > Diego Nieto Cid, on Wed 04 Nov 2015 10:50:35 -0300, wrote:
> > > >   assert (__hurd_local_reply_port == arg || arg ==
> MACH_PORT_NULL)
> > > >
> > > > AIUI any other values are bogus given how 'mig_get_reply_port'
> and
> > > > 'mig_dealloc_reply_port' are meant to be paired.
> > >
> > > That's probably a good thing to do, yes.
> >
> > What's wrong with?
> > mach_port_t port = __hurd_local_reply_port;
> > assert (port == arg || arg == MACH_PORT_NULL)
> >
> 
> That's ok. I just rewrote it to make clear what the accepted values
> were. Because the second patch incorporated an if statement that
> converted the suggested condition in a tautology.

I agree, the #if 1 part was not OK. What about rewriting
__mig_get_reply_port() too to be more readable?

Please tell me if this patch is faulty in some way, I'm still on the
learning curve.

Index: glibc-2.19/sysdeps/mach/hurd/mig-reply.c
===================================================================
--- glibc-2.19.orig/sysdeps/mach/hurd/mig-reply.c
+++ glibc-2.19/sysdeps/mach/hurd/mig-reply.c
@@ -26,12 +26,13 @@ mach_port_t __hurd_reply_port0;
 mach_port_t
 __mig_get_reply_port (void)
 {
-  if (__hurd_local_reply_port == MACH_PORT_NULL ||
-      (&__hurd_local_reply_port != &__hurd_reply_port0
-       && __hurd_local_reply_port == __hurd_reply_port0))
-    __hurd_local_reply_port = __mach_reply_port ();
+  mach_port_t port = __hurd_local_reply_port;
+  if (port == MACH_PORT_NULL ||
+      (&__hurd_local_reply_port != &__hurd_reply_port0 &&
+       port == __hurd_reply_port0))
+    port = __mach_reply_port ();
 
-  return __hurd_local_reply_port;
+  return port;
 }
 weak_alias (__mig_get_reply_port, mig_get_reply_port)
 
@@ -40,6 +41,7 @@ void
 __mig_dealloc_reply_port (mach_port_t arg)
 {
   mach_port_t port = __hurd_local_reply_port;
+  assert (port == arg || arg == MACH_PORT_NULL);
   __hurd_local_reply_port = MACH_PORT_NULL;    /* So the mod_refs
RPC won't use it.  */
 
   if (MACH_PORT_VALID (port))

Samuel: Regarding your previous comment:
>> Additionally, any strong reason to not change mig??
> 
> Having to deal with the introduced incompatibility.

Incompatibility with what, older versions of glibc/hurd/mach/mig, which
ones? How many users are there of GNU/Hurd, especially running old
releases? Maybe I can think of one: antrik, but he is capable enough to
cope with the changes.