From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 8E4742C1AB for ; Thu, 18 Apr 2019 11:16:43 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 9IHx8jfYjqUo for ; Thu, 18 Apr 2019 11:16:43 -0400 (EDT) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 462BA2BC3C for ; Thu, 18 Apr 2019 11:16:43 -0400 (EDT) Received: by smtp59.i.mail.ru with esmtpa (envelope-from ) id 1hH8mH-0003EV-Ek for tarantool-patches@freelists.org; Thu, 18 Apr 2019 18:16:41 +0300 Date: Thu, 18 Apr 2019 18:16:41 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH 1/4] swim: move sockaddr_in checkers to swim_proto.h Message-ID: <20190418151641.GC13022@chai> References: <1e3490a96e2daec7906def26f31265d995966204.1555530516.git.v.shpilevoy@tarantool.org> <77eb864c-bb05-4f6d-ec96-b784421b83d8@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <77eb864c-bb05-4f6d-ec96-b784421b83d8@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org * Vladislav Shpilevoy [19/04/18 17:11]: > Sorry, accidentally broke a test. The fix is > force pushed: > > ====================================================== > swim_inaddr_is_empty(const struct sockaddr_in *addr) > { > - return addr->sin_port == 0 && addr->sin_addr.s_addr == 0; > + return addr->sin_port == 0 || addr->sin_addr.s_addr == 0; > } This means you haven't paused to explain or test or add a pre-condition check to the setter function. Why either of the components can be empty, not both? Shouldn't you check for such broken addresses when setting them, to not allow setting them at all? The patch is OK to push obviously, just food for thought. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov