Re: [tarantool-patches] Re: [PATCH 09/11] evio: refactoring

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 03/12/2018 19:37, Vladimir Davydov wrote:
> On Fri, Nov 30, 2018 at 06:39:41PM +0300, Vladislav Shpilevoy wrote:
>> Remove useless, wrong and obvious comments. Correct
>> code and comment style. Reuse some code, unnecessary
>> try/catch.
>> ---
>>   src/evio.cc | 186 ++++++++++++++++++++--------------------------------
>>   src/evio.h  |  40 ++++++-----
>>   2 files changed, 89 insertions(+), 137 deletions(-)
>>
>> diff --git a/src/evio.cc b/src/evio.cc
>> index 25f699bab..166ba7f95 100644
>> --- a/src/evio.cc
>> +++ b/src/evio.cc
>> @@ -29,7 +29,6 @@
>>    * SUCH DAMAGE.
>>    */
>>   #include "evio.h"
>> -#include "uri.h"
>>   #include "scoped_guard.h"
>>   #include <stdio.h>
>>   #include <sys/socket.h>
>> @@ -41,41 +40,33 @@
>>   #include <trivia/util.h>
>>   #include "exception.h"
>>   
>> -static void
>> -evio_setsockopt_server(int fd, int family, int type);
>> -
>> -/** Note: this function does not throw. */
>>   void
>>   evio_close(ev_loop *loop, struct ev_io *evio)
>>   {
>>   	/* Stop I/O events. Safe to do even if not started. */
>>   	ev_io_stop(loop, evio);
>> -	/* Close the socket. */
>> -	close(evio->fd);
>> -	/* Make sure evio_has_fd() returns a proper value. */
>> -	evio->fd = -1;
>> +	if (evio_has_fd(evio)) {
>> +		close(evio->fd);
>> +		ev_io_set(evio, -1, 0);
>> +	}
>>   }
>>   
>> -/**
>> - * Create an endpoint for communication.
>> - * Set socket as non-block and apply protocol specific options.
>> - */
>>   void
>> -evio_socket(struct ev_io *coio, int domain, int type, int protocol)
>> +evio_socket(struct ev_io *evio, int domain, int type, int protocol)
>>   {
>> -	assert(coio->fd == -1);
>> -	coio->fd = sio_socket(domain, type, protocol);
>> -	if (coio->fd < 0)
>> +	assert(! evio_has_fd(evio));
>> +	evio->fd = sio_socket(domain, type, protocol);
>> +	if (evio->fd < 0)
>>   		diag_raise();
>> -	if (evio_setsockopt_client(coio->fd, domain, type) != 0) {
>> -		close(coio->fd);
>> -		coio->fd = -1;
>> +	if (evio_setsockopt_client(evio->fd, domain, type) != 0) {
>> +		close(evio->fd);
>> +		ev_io_set(evio, -1, 0);
>>   		diag_raise();
>>   	}
>>   }
>>   
>>   static int
>> -evio_setsockopt_keepalive(int fd)
>> +evio_setsockopt_keeping(int fd)
>>   {
>>   	int on = 1;
>>   	/*
> 
> Refactoring/renaming done by this patch looks dubious to me. It isn't
> worth polluting the history. Please drop it.
> 

As you wish. See v2.