Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753533AbbBHODS (ORCPT ); Sun, 8 Feb 2015 09:03:18 -0500 Received: from mail-la0-f46.google.com ([209.85.215.46]:38107 "EHLO mail-la0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751301AbbBHODQ (ORCPT ); Sun, 8 Feb 2015 09:03:16 -0500 MIME-Version: 1.0 In-Reply-To: <54D7329C.206@users.sourceforge.net> References: <530CD2C4.4050903@users.sourceforge.net> <530CF8FF.8080600@users.sourceforge.net> <530DD06F.4090703@users.sourceforge.net> <5317A59D.4@users.sourceforge.net> <54CF93BB.40206@users.sourceforge.net> <54CF9666.5060407@users.sourceforge.net> <20150206211234.GA14803@redhat.com> <54D7329C.206@users.sourceforge.net> From: Mike Snitzer Date: Sun, 8 Feb 2015 09:02:53 -0500 X-Google-Sender-Auth: Xg7RPF2ivlzJuF3ywMx17QBinTE Message-ID: Subject: Re: [PATCH 3/3] dm ioctl: Delete an unnecessary check before the function call "dm_table_destroy" To: SF Markus Elfring Cc: Alasdair Kergon , Neil Brown , device-mapper development , "linux-raid@vger.kernel.org" , Julia Lawall , kernel-janitors@vger.kernel.org, LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1403 Lines: 30 On Sun, Feb 8, 2015 at 4:55 AM, SF Markus Elfring wrote: >> Your proposed patch (while technically correct) hurts code clarity. > > How many source code readability and understanding challenges does each > additional condition check cause? Please don't make a mountain out of a mole hill in an attempt to defend your robotic patch (I'm quite tired of some of these static analyzer patch submissions). FYI, I did stage your other patch for 3.20, see: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-for-3.20&id=d0ce7e911c97c7c6df1081dcedfefced82a0c6bf > Can the affected place become also a bit more efficient? Efficiency isn't a concern in this instance (it isn't a hot IO path). And even if it were, a branch (with current code) is more efficient vs a a jump + branch (your proposed patch) -- in the case that no active table exists. Now if it likely that old_map does exist then yes your patch is always a very slight win. But given the duality of the calling function (deals with loading a new map and destroying the old map if it exists) I prefer to keep the code as is. Sorry. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/