Inconsistent code found by static analyzers at base_events.py

[Anchels]

Greetings! I've been analyzing Cpython with Svace static analyzer. It has found a code inconsitency issue at the asyncio library in the following method:

cpython/Lib/asyncio/base_events.py

Lines 1653 to 1681 in 7ebbd27

	async def connect_accepted_socket(
	self, protocol_factory, sock,
	*, ssl=None,
	ssl_handshake_timeout=None,
	ssl_shutdown_timeout=None):
	if sock.type != socket.SOCK_STREAM:
	raise ValueError(f'A Stream Socket was expected, got {sock!r}')
	if ssl_handshake_timeout is not None and not ssl:
	raise ValueError(
	'ssl_handshake_timeout is only meaningful with ssl')
	if ssl_shutdown_timeout is not None and not ssl:
	raise ValueError(
	'ssl_shutdown_timeout is only meaningful with ssl')
	if sock is not None:
	_check_ssl_socket(sock)
	transport, protocol = await self._create_connection_transport(
	sock, protocol_factory, ssl, '', server_side=True,
	ssl_handshake_timeout=ssl_handshake_timeout,
	ssl_shutdown_timeout=ssl_shutdown_timeout)
	if self._debug:
	# Get the socket from the transport because SSL transport closes
	# the old socket and creates a new SSL socket
	sock = transport.get_extra_info('socket')
	logger.debug("%r handled: (%r, %r)", sock, transport, protocol)
	return transport, protocol

The problem is a redundant comparison with a None value for reference sock here:

cpython/Lib/asyncio/base_events.py

Lines 1669 to 1670 in 7ebbd27

	if sock is not None:
	_check_ssl_socket(sock)

which has already been dereferenced before:

cpython/Lib/asyncio/base_events.py

Lines 1658 to 1659 in 7ebbd27

	if sock.type != socket.SOCK_STREAM:
	raise ValueError(f'A Stream Socket was expected, got {sock!r}')

According to the documentation, the sock input parameter is a preexisting object returned from the socket.accept() function. So it shouldn't be None

---

Proposed solution

I believe it is advisable to either move the null check for sock to the beginning of the function, or remove it. I'm not sure which of these options would be more in line with the regulations. After I get a response, I might be able to create a pull request.

Linked PRs

• gh-132307: Remove unnecessary check in asyncio/base_events.py #132324

[ZeroIntensity]

I'm pretty sure we can just remove the sock is not None check.

[sobolevn]

But, should we really? Is there any indication that something does not work? Or that this check is slow?

[ZeroIntensity]

It's just dead code. That check can never fail, because if it was None, an error would have already been raised when accessing type. If it's messing with static analysis, then I don't see why it should stay.

[kumaraditya303]

I think it is fine to remove it although it doesn't matter much.