Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] wireguard: queueing: Fix implicit type conversion
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>, jiasheng@iscas.ac.cn
Cc:     David Miller <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        WireGuard mailing list <wireguard@lists.zx2c4.com>,
        Netdev <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
References: <1635469664-1708957-1-git-send-email-jiasheng@iscas.ac.cn>
 <CAHmME9p9EA0qCw2ha_X9HR7NWSt1Zam4+srYHCs=-U4LvQiJdA@mail.gmail.com>
From:   Eric Dumazet <eric.dumazet@gmail.com>
Message-ID: <99a32425-ca42-8d99-1276-efb889300cce@gmail.com>
Date:   Fri, 29 Oct 2021 08:30:26 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <CAHmME9p9EA0qCw2ha_X9HR7NWSt1Zam4+srYHCs=-U4LvQiJdA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 10/29/21 7:27 AM, Jason A. Donenfeld wrote:
> On Fri, Oct 29, 2021 at 3:08 AM Jiasheng Jiang <jiasheng@iscas.ac.cn> wrote:
>> It is universally accepted that the implicit type conversion is
>> terrible.
> 
> I'm not so sure about this, but either way, I think this needs a bit
> more justification and analysis to merge. cpumask_weight returns an
> unsigned, for example, and is used as a modulo operand later in the
> function. It looks like nr_cpumask_bits is also unsigned. And so on.
> So you're really trading one implicit type conversion package for
> another. If you're swapping these around, why? It can't be because,
> "it is universally accepted that the implicit type conversion is
> terrible," since you're adding more of it in a different form. Is your
> set of implicit type conversions semantically more proper? If so,
> please describe that. Alternatively, is there a way to harmonize
> everything into one type? Is there a minimal set of casts that enables
> that?
>

I agree with you.

Even standard iterators play/mix with signed/unsigned in plain sight.

extern unsigned int nr_cpu_ids;

unsigned int cpumask_next(int n, const struct cpumask *srcp);

int cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool wrap);

#define for_each_cpu(cpu, mask)				\
	for ((cpu) = -1;				\
		(cpu) = cpumask_next((cpu), (mask)),	\
		(cpu) < nr_cpu_ids;)