[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Confusing definitions and declarations of mig_dealloc_reply_port()
From: |
Svante Signell |
Subject: |
Re: Confusing definitions and declarations of mig_dealloc_reply_port() |
Date: |
Wed, 04 Nov 2015 10:30:44 +0100 |
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?
- Confusing definitions and declarations of mig_dealloc_reply_port(), Svante Signell, 2015/11/03
- Message not available
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(),
Svante Signell <=
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Diego Nieto Cid, 2015/11/04
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Samuel Thibault, 2015/11/04
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Svante Signell, 2015/11/04
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Samuel Thibault, 2015/11/04
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Diego Nieto Cid, 2015/11/04
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Svante Signell, 2015/11/05
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Pino Toscano, 2015/11/05
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Svante Signell, 2015/11/05
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Samuel Thibault, 2015/11/06
- Re: Confusing definitions and declarations of mig_dealloc_reply_port(), Svante Signell, 2015/11/06