From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E1A6D45B04; Thu, 10 Oct 2024 14:35:53 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B311B4042C; Thu, 10 Oct 2024 14:35:53 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 91DD1402E9 for ; Thu, 10 Oct 2024 14:35:51 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 45B001F024 for ; Thu, 10 Oct 2024 14:35:51 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 392C11EFDD; Thu, 10 Oct 2024 14:35:51 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.2 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.2 Received: from [192.168.1.85] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 570711EEF3; Thu, 10 Oct 2024 14:35:47 +0200 (CEST) Message-ID: Date: Thu, 10 Oct 2024 14:35:46 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v12 6/7] eal: add unit tests for atomic bit access functions To: David Marchand Cc: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , dev@dpdk.org, Heng Wang , Stephen Hemminger , Tyler Retzlaff , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Jack Bond-Preston , Chengwen Feng , Thomas Monjalon References: <20240920062437.738706-2-mattias.ronnblom@ericsson.com> <20240920104754.739033-1-mattias.ronnblom@ericsson.com> <20240920104754.739033-7-mattias.ronnblom@ericsson.com> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2024-10-10 14:14, David Marchand wrote: > On Thu, Oct 10, 2024 at 1:56 PM Mattias Rönnblom wrote: >> OK. Nothing obvious from what I can see in the code. Unrelated: why did >> you remove all empty lines in the "template" macros? Makes them much >> harder to read. > > Those macros are hard to read. > Even the more reason to make them easier to read. > There was an extra indent that resulted in splitting many lines. They were just formatted with the same indentation as all other macros, no? So not "extra". That said, the indentation leaves less room on for the actual code, so I have no issue with the removal of the standard multi-line macro indentation, although it wouldn't have hurt to have pointed this out earlier, so we wouldn't have to have this discussion after-the-fact. > So I started with removing this extra indent. > > Then, the trailing tabs (with some pseudo randomly mixed in spaces) > were replaced with a single space before the \ for line continuation. > This enhances code review for update: with a single systematic space, > there is no need to consider if you must update all the context when > adding a line longer than what was already present. DPDK uses both the kernel-style multi-line macro formatting, and what you describe above. Your argument above makes sense, but I also find the kernel style more visually appealing. > > In the end, I was left with cases like: > \ > \ > \ > > And I preferred to remove this extra line as it did not enhance the > clarity of those macros. > > \ \ \ Would have been visually more appealing, but less consistent. To me, removing these delimiting empty lines is no different from doing the same in a regular C function. Empty lines are delimiters, there to group related statements, and they serve a purpose. Now, the macros look like they are written by someone who doesn't care about readability.