write_ports delfd case

write_ports delfd case

[J. Bruce Fields]

Does anyone know what __write_ports_delfd() is meant to do?

The block comment above write_ports claims it handles writes of the form
"-<file descriptor>", which makes no sense (the file table of the writer
has nothing to do with anything).  It's called only when the character
after the "-" is a digit, but the names it matches against (generated by
svc_one_sock_name()) start with "ipv4" or "ipv6".

I guess we should just rip out that case entirely?  Or maybe I'm missing
something.

--b.

Re: write_ports delfd case

[Chuck Lever]

On 07/19/10 03:21 PM, J. Bruce Fields wrote:
> Does anyone know what __write_ports_delfd() is meant to do?
>
> The block comment above write_ports claims it handles writes of the form
> "-<file descriptor>", which makes no sense (the file table of the writer
> has nothing to do with anything).  It's called only when the character
> after the "-" is a digit, but the names it matches against (generated by
> svc_one_sock_name()) start with "ipv4" or "ipv6".

I suspect the comment above write_ports() is not correct.  I assumed 
that delfd was symmetrical with addfd, but it isn't.  More likely, addfd 
returns a string name that can be passed to write_ports (with a 
preceding '-') to terminate the socket.

Re: write_ports delfd case

[Jeff Layton]

On Mon, 19 Jul 2010 15:32:57 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> On 07/19/10 03:21 PM, J. Bruce Fields wrote:
> > Does anyone know what __write_ports_delfd() is meant to do?
> >
> > The block comment above write_ports claims it handles writes of the form
> > "-<file descriptor>", which makes no sense (the file table of the writer
> > has nothing to do with anything).  It's called only when the character
> > after the "-" is a digit, but the names it matches against (generated by
> > svc_one_sock_name()) start with "ipv4" or "ipv6".
> 
> I suspect the comment above write_ports() is not correct.  I assumed 
> that delfd was symmetrical with addfd, but it isn't.  More likely, addfd 
> returns a string name that can be passed to write_ports (with a 
> preceding '-') to terminate the socket.

Yeah -- seems broken to me. Given that it clearly doesn't work and I'm
not aware of anyone having complained, perhaps it would be best to
remove it?

-- 
Jeff Layton <jlayton@redhat.com>

Re: write_ports delfd case

[Neil Brown]

On Mon, 19 Jul 2010 16:07:03 -0400
Jeff Layton <jlayton@redhat.com> wrote:

> On Mon, 19 Jul 2010 15:32:57 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> > On 07/19/10 03:21 PM, J. Bruce Fields wrote:
> > > Does anyone know what __write_ports_delfd() is meant to do?
> > >
> > > The block comment above write_ports claims it handles writes of the form
> > > "-<file descriptor>", which makes no sense (the file table of the writer
> > > has nothing to do with anything).  It's called only when the character
> > > after the "-" is a digit, but the names it matches against (generated by
> > > svc_one_sock_name()) start with "ipv4" or "ipv6".
> > 
> > I suspect the comment above write_ports() is not correct.  I assumed 
> > that delfd was symmetrical with addfd, but it isn't.  More likely, addfd 
> > returns a string name that can be passed to write_ports (with a 
> > preceding '-') to terminate the socket.
> 
> Yeah -- seems broken to me. Given that it clearly doesn't work and I'm
> not aware of anyone having complained, perhaps it would be best to
> remove it?
> 

Looks like 
   a217813f9067b785241cb7f31956e51d2071703a

was the offending commit.  Adding the "isdigit" test is just wrong.

It is just *wrong* in other ways too.  You really should have a host address
as well as a port number to identify a socket endpoint, even if it is the
wildcard address.

Oh well.....

NeilBrown