From: Jason Gunthorpe <jgg@mellanox.com>
To: David Miller <davem@davemloft.net>, "g@mellanox.com" <g@mellanox.com>
Cc: "torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
"dledford@redhat.com" <dledford@redhat.com>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: Annoying gcc / rdma / networking warnings
Date: Mon, 13 May 2019 18:30:58 +0000 [thread overview]
Message-ID: <20190513183053.GI7948@mellanox.com> (raw)
In-Reply-To: <20190512.201701.1918995863082655897.davem@davemloft.net>
On Sun, May 12, 2019 at 08:17:01PM -0700, David Miller wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
> Date: Mon, 13 May 2019 01:11:42 +0000
>
> > I think the specific sockaddr types should only ever be used if we
> > *know* the sa_family is that type. If the sa_family is not known then
> > it should be sockaddr or sockaddr_storage. Otherwise things get very
> > confusing.
> >
> > When using sockaddr_storage code always has the cast to sockaddr
> > anyhow, as it is not a union, so this jaunty cast is not out of place
> > in sockets code.
>
> From what I can see, each and every call side of these helpers like
> rdma_gid2ip() et al. redefine this union type over and over and over
> again in the local function.
Yes, the repeated union is very ugly and could be consolidated - or
should just use sockaddr_storage in the first place.
> It seems that if we just defined it explicitly in one place, like
> include/rdma/ib_addr.h, then we could have tdma_gid2ip(), addr_resolve(),
> and rdma_resolve_ip() take that type explcitily.
I pulled on this thread for a while and the number of places that
would need to convert to use a global 'union rdma_sockaddr_inet'
started to become pretty silly and weird. I eventually reached a point
where I had to cast a sockaddr * to the union - which is something
that makes no sense and I gave up.
So, I think this feels simpler to follow the usual sockaddr_storage
pattern and only use the union to declare the initial storage. Then
everything else just uses the sockaddr * plus casts..
Jason
prev parent reply other threads:[~2019-05-13 18:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-11 16:52 Annoying gcc / rdma / networking warnings Linus Torvalds
2019-05-11 20:02 ` Sowmini Varadhan
2019-05-13 1:11 ` Jason Gunthorpe
2019-05-13 1:11 ` Jason Gunthorpe
2019-05-13 3:17 ` David Miller
2019-05-13 3:17 ` David Miller
2019-05-13 18:30 ` Jason Gunthorpe [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190513183053.GI7948@mellanox.com \
--to=jgg@mellanox.com \
--cc=davem@davemloft.net \
--cc=dledford@redhat.com \
--cc=g@mellanox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.