Re: [Qemu-devel] [PATCH 06/18] slirp: Factorizing and cleaning solookup()

[Samuel Thibault <samuel.thibault@gnu.org>]

Samuel Thibault, on Fri 11 Dec 2015 20:29:44 +0100, wrote:
> Thomas Huth, on Fri 11 Dec 2015 16:06:22 +0100, wrote:
> > On 11/12/15 01:15, Samuel Thibault wrote:
> > > From: Guillaume Subiron <maethor@subiron.org>
> > > 
> > > This patch makes solookup() compatible with varying address families. Also,
> > > this function was only compatible with TCP. Having the socket list in
> > > argument, it is now compatible with UDP too. Finally, some optimization
> > > code is factorized inside the function (the function look at the last
> > > returned result before browsing the complete socket list).
> > > 
> > > This also adds a sockaddr_equal() function to compare two
> > > sockaddr_storage.
> > 
> > I'd maybe also split this patch into two - first introduce the
> > sockaddr_equal() function, then do the other changes. If you do too much
> > stuff in one patch, it gets more difficult to read.
> 
> Actually I think I'll do the converse. solookup doesn't take a sockaddr
> first, so introducing sockaddr_equal just to make it take separate
> in_addr and lport would make both patches clumsy. Making solookup access
> sockaddr first is however not too bad.

Mmm, that's still clumsy: making solookup use sockaddr_storage
without using solookup means putting all the casts there, it's really
unreadable, while the version with sockaddr_equal is nice to read. And
conversely, introducing sockaddr_equal first is clumsy.

I'll however have a look at introducing the optimizations etc. first, so
that both making solookup use sockaddr and introducing sockaddr_equal
looks natural.

Samuel