netdev - Re: [Patch net-next v2 5/8] sunrpc: use generic union inet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130806062801.67714276@tlielax.poochiereds.net>
Date:	Tue, 6 Aug 2013 06:28:01 -0400
From:	Jeff Layton <jlayton@...hat.com>
To:	Cong Wang <amwang@...hat.com>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Trond Myklebust <Trond.Myklebust@...app.com>,
	"J. Bruce Fields" <bfields@...ldses.org>, linux-nfs@...r.kernel.org
Subject: Re: [Patch net-next v2 5/8] sunrpc: use generic union inet_addr

On Mon, 05 Aug 2013 11:14:05 +0800
Cong Wang <amwang@...hat.com> wrote:

> On Fri, 2013-08-02 at 09:36 -0400, Jeff Layton wrote:
> > On Fri,  2 Aug 2013 15:14:31 +0800
> > Cong Wang <amwang@...hat.com> wrote:
> > 
> > > From: Cong Wang <amwang@...hat.com>
> > > 
> > > sunrpc defines some helper functions for sockaddr, actually they
> > > can re-use the generic functions for union inet_addr too.
> > 
> > Only some of these patches in this series have made it to lists to
> > which I'm subscribed, so I may be missing some context here...
> > 
> > I'm not sure I really understand the value of "union inet_addr". Why
> > not just use the conventional method of passing around "struct sockaddr"
> > pointers, and then casting them to struct sockaddr_in/sockaddr_in6
> > depending on what the sa_family is set to?
> 
> Yes.
> 
> > 
> > With that you wouldn't need to leave the (now pointless) rpc_* wrappers
> > in place and could just call your new helpers directly.
> > 
> 
> J. Bruce asked the same question, see:
> http://marc.info/?l=linux-nfs&m=137475685903460&w=2
> 

Not exactly...

He asked why you were keeping the wrappers and you replied that it was
because you didn't want to change all of the callers of rpc_get_port.
That's a fair enough answer to his question.

My question is a bit more fundamental: Why are you using this new union
in your patches instead of simply passing around "struct sockaddr"
pointers? If you did that, then you could simply replace all of the
rpc_* wrappers with your generic ones, since you wouldn't need to do
the cast to this (seemingly unnecessary) union.

FWIW, I too am happy to see these routines moved to common code. I just
wonder whether it might make more sense to use the existing convention
instead of this new union.

-- 
Jeff Layton <jlayton@...hat.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html