From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933153AbbCPW6u (ORCPT ); Mon, 16 Mar 2015 18:58:50 -0400 Received: from mail-la0-f54.google.com ([209.85.215.54]:33115 "EHLO mail-la0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754318AbbCPW6t (ORCPT ); Mon, 16 Mar 2015 18:58:49 -0400 Message-ID: <55076025.3060109@gmail.com> Date: Mon, 16 Mar 2015 23:58:45 +0100 From: Mateusz Kulikowski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Joe Perches CC: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 3/5] staging: rtl8192e: fix coding style errors (macros in parentheses) References: <1426451987-24519-1-git-send-email-mateusz.kulikowski@gmail.com> <1426451987-24519-4-git-send-email-mateusz.kulikowski@gmail.com> <1426481460.2678.35.camel@perches.com> In-Reply-To: <1426481460.2678.35.camel@perches.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16.03.2015 05:51, Joe Perches wrote: > On Sun, 2015-03-15 at 21:39 +0100, Mateusz Kulikowski wrote: >> Fix checkpatch.pl errors 'Macros with complex values should be enclosed in parentheses'. > [] >> diff --git a/drivers/staging/rtl8192e/rtl819x_HT.h b/drivers/staging/rtl8192e/rtl819x_HT.h > [] >> @@ -78,7 +78,7 @@ enum chnl_op { >> }; >> >> #define CHHLOP_IN_PROGRESS(_pHTInfo) \ >> - ((_pHTInfo)->ChnlOp > CHNLOP_NONE) ? true : false >> + (((_pHTInfo)->ChnlOp > CHNLOP_NONE) ? true : false) > > This would perhaps be better without the ternary as > the compiler might not optimize the ternary away > > #define CHHLOP_IN_PROGRESS(_pHTInfo) \ > ((_pHTInfo)->ChnOp > CHNLOP_NONE) > > This would also probably be better as a static inline > not a macro, but as this is never used it's actually > better just to remove it. Thanks for the hints, done in v5 (other macros were also harmed) > >> @@ -385,8 +385,8 @@ extern u8 MCS_FILTER_1SS[16]; >> #define LEGACY_WIRELESS_MODE IEEE_MODE_MASK >> >> #define CURRENT_RATE(WirelessMode, LegacyRate, HTRate) \ >> - ((WirelessMode & (LEGACY_WIRELESS_MODE)) != 0) ? \ >> - (LegacyRate) : (PICK_RATE(LegacyRate, HTRate)) >> + (((WirelessMode & (LEGACY_WIRELESS_MODE)) != 0) ? \ >> + (LegacyRate) : (PICK_RATE(LegacyRate, HTRate))) > > This would be better with the various macro arguments > parenthesized (especially WirelessMode) > > #define CURRENT_RATE(WirelessMode, LegacyRate, HTRate) \ > (((WirelessMode) & LEGACY_WIRELESS_MODE) ? \ > (LegacyRate) : PICK_RATE((LegacyRate), HTRate)) > > As this is used exactly once, it's probably better > expanded in that one place and the macro removed. > > That's true for PICK_RATE as well. Will replace with static function (in .c file) - code that uses this macro is nested enough Regards Mateusz