[prev in list] [next in list] [prev in thread] [next in thread]
List: hurd-bug
Subject: Re: Confusing definitions and declarations of mig_dealloc_reply_port()
From: Svante Signell <svante.signell () gmail ! com>
Date: 2015-11-04 9:30:44
Message-ID: 1446629444.31749.66.camel () gmail ! com
[Download RAW message or body]
Diego,
Cc: bug-hurd.
On Tue, 2015-11-03 at 15:55 -0300, Diego Nieto Cid wrote:
> Hi
>
> 2015-11-03 9:51 GMT-03:00 Svante Signell <svante.signell@gmail.com>:
> >
> > Hello,
> >
>
> Definition, declaration and usages are all consistent, they pass and
> receive one argument.
> The argument being unsed is just an implementation detail; earlier
> versions may had used
> it but was never removed [*]....
>
> I guess what's important is that it's consistent with how
> mig_get_reply_port gives the port
> to its users, right?
>
> > __mig_dealloc_reply_port (MACH_PORT_NULL);
> >
>
> This use case would be broken by...
>
> ...this patch. arg would be MACH_PORT_NULL and no refs would get
> released, leaking the reply port.
Well, the following patch would resolve that, right? Maybe overkill
though.
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
@@ -39,7 +39,16 @@ weak_alias (__mig_get_reply_port, mig_ge
void
__mig_dealloc_reply_port (mach_port_t arg)
{
+#if 1
+ mach_port_t port;
+ if (arg == MACH_PORT_NULL)
+ port = __hurd_local_reply_port;
+ else
+ port = arg;
+#else
mach_port_t port = __hurd_local_reply_port;
+#endif
+ 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))
> I'd suggest to assert (port == arg || arg == MACH_PORT_NULL) just to
> be sure users don't expect
> other port to be deallocated, which would be a bug.
See above.
> > Another solution could be to remove the argument in the
> > mig_dealloc_reply_port definition, but that would require a change
> also
> > to mig.
> >
>
> [*]...probably because of this :)
Why not change mig, is it holy?
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic