Re: [Tarantool-patches] [PATCH] Work-around WSL assert when SO_LINGER is set on unix sockets

["Timur Safin" <tsafin@tarantool.org>]

: -----Original Message-----
: From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
: Subject: Re: [PATCH] Work-around WSL assert when SO_LINGER is set on unix
: sockets
: 


: 
: 1. Please, add a subsystem prefix to the commit title. For
: examples see other commits in the repository.

Here I need a help, because evidence is not apparent. The last commit to 
evio.c was not marked with subsystem 

	tsafin@M1BOOK6319:~/tarantool$ git log --oneline src/lib/core/evio.c
	835d22aa0 (HEAD -> tsafin/gh-4659-wsl-no-linger-assert, master)
		Work-around WSL assert when SO_LINGER is set on unix sockets
	3f5f59bb5 Move 'core' and 'uuid' libs to src/lib

Should it be `core` or `coio`?

: > Branch: https://github.com/tarantool/tarantool/tree/tsafin/gh-4659-wsl-
: no-linger-assert
: 
: 2. Please, provide both branch and issue links.
: 4. The patch has nothing to do with gh-4659:
: https://github.com/tarantool/tarantool/issues/4659
: "sql: raise an error in case space features HASH index".

This is the question - what is the current practice for such 
simplistic patches? SOP says there is no need to open issue 
so I'm marking branch with github issue # of which this 
patch is byproduct. 

:: 3. I see, that on the branch your commit message is just empty.
: Seems like you didn't push the latest message.

Yup, I didn't fully realize that such single-patch patchsets 
should contain patch commit messages and not that separate messages 
which I'd put to the cover message of patchset. Will amend.
 
: 
: > ---
: 
: 5. Links should be below this marker '---'. Some people apply
: patches from emails, and when you write links above '---', you
: make them part of the commit message.

Yes, thanks for the correction, this is side-effect of an approach
used above.
 
: > -	/* Send all buffered messages on socket before take
: > -	 * control out from close(2) or shutdown(2). */
: > -	struct linger linger = { 0, 0 };
: > +	if (family != AF_UNIX) {
: 
: 6. Is there any proof that it is no-op on Linux for AF_UNIX?
:

Good question! Never had enough time and motivation to proof this assumption.
Now you've asked I'll look into linux kernel tcp implementation. Stay ktuned.
 
 
: I would rather call sio_setsockopt() always. And ignore an
: error, if it is EINVAL for AF_UNIX.

Probably it's less damaging approach. I'll probably use it - but first I need to
Look around in the kernel.


: 
: 7. It is worth adding a comment why SO_LINGER is workarounded
: somehow.

Indeed. Will do .

: 
: > +		/* Send all buffered messages on socket before
: > +		 * take control out from close(2) or shutdown(2). */
: > +		struct linger linger = { 0, 0 };
: > +
: > +		if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER,
: > +				&linger, sizeof(linger)))
: > +			return -1;
: > +	}


Timur