Re: [PATCH ucspi-vsock 7/7] Extract vsockserver-socketbinder and vsockserverd

Alyssa Ross
2021-03-21T14:39:13Z

[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]

On Sun, Mar 21, 2021 at 02:33:14PM +0000, Alyssa Ross wrote:
> On Sat, Mar 20, 2021 at 07:52:00PM -0700, Cole Helbling wrote:
> > On Sat Mar 20, 2021 at 1:24 PM PDT, Alyssa Ross wrote:
> > > On Thu, Mar 18, 2021 at 08:39:31PM -0700, Cole Helbling wrote:
> > > > > + if (listen(fd ,40) == -1)
> > > >
> > > > Minor formatting nit (comma, then space); but also, what is `40`
> > > > representative of? Should this be `#define`d, or otherwise assigned to
> > > > some descriptive name?
> > >
> > > Looks like the next thing I should do after this is set up clang-tidy or
> > > something. ;)
> > >
> > > It's the backlog parameter, i.e. the minimum[1] number of connections
> > > the kernel should allow to queue up before it starts rejecting them.
> > >
> > > Would the following additional diff make you happy? :)
> > > (I won't bother resending the whole patch with it applied; for the
> > > purposes of reviewing just pretend this is part of the patch I sent.)
> >
> > Yep, looks good to me! Though if I'm being greedy, maybe add the above
> > explanation as a comment above the new variable? ("It's the backlog
> > [...]")
>
> I think I don't want to get too much into explaining things listen(2)
> will explain better than me (but you were right about giving this
> number a name)!
>
> > Entire series looks good as well:
> >
> > Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
>
> Thanks!
>
> To ssh://atuin/home/spectrum/git/ucspi-vsock.git
> e718a97..a452f16 master -> master

Actually it's e718a97..20a27f1. Realised I hadn't actually applied that
diff, and decided to just force push before anybody noticed. ;)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]