Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e2cf72f1-7fe4-77d7-6700-015fa5214abc@vivo.com>
Date:   Thu, 10 Mar 2022 16:48:07 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.1
Subject: Re: [PATCH] irqchip/apple-aic: application of sizeof() to a pointer
Content-Language: en-US
To:     Marc Zyngier <maz@kernel.org>
Cc:     Hector Martin <marcan@marcan.st>, Sven Peter <sven@svenpeter.dev>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>,
        Thomas Gleixner <tglx@linutronix.de>,
        "moderated list:ARM/APPLE MACHINE SUPPORT" 
        <linux-arm-kernel@lists.infradead.org>,
        "open list:IRQCHIP DRIVERS" <linux-kernel@vger.kernel.org>,
        "zhengkui_guo@outlook.com" <zhengkui_guo@outlook.com>
References: <20220310050238.4478-1-guozhengkui@vivo.com>
 <87sfrqyzyr.wl-maz@kernel.org>
From:   Guo Zhengkui <guozhengkui@vivo.com>
In-Reply-To: <87sfrqyzyr.wl-maz@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VEI5L3llcENZaFZha0RldTZVQUhEazJDWTdCdzlBa2lkSUVKb1lPbDV4RDcw?=
 =?utf-8?B?bVJFU2tjdUNhSU5weEg2OVNHSjh2T2hjOGdXa0Q2OWVHVVZFZDhxNVhQNXov?=
 =?utf-8?B?OXRDd3pSd2hCa2U2R1pTSUp3YWUrWldpVmVlckxiVWE3eXA2R21YT3ZmTGp6?=
 =?utf-8?B?blpWbDl2S1JSMGR5YVBZN2lmcFZpbjBsTEFRS0N1d2hqSTI4VU94ZGdCRTVB?=
 =?utf-8?B?QU1SWnlIZkd3OW1uL2NLM2hWRnNjbHprVnBseGNaYkhZbkJ0MFBqRXlkcnBy?=
 =?utf-8?B?aEpDc0I5K2xOWXBNbk9VLzFwNWJDZVFxSGRxQmpxdGM4ZndreURlTXBHOWth?=
 =?utf-8?B?Sk1pZWdaZ0FQbnhtSkJ2bXl5bmp3TzFMWEQvTmw1QmNHR0kvZ0lqVHpZUG1K?=
 =?utf-8?B?c001dFBnUVZrRXRxUHlaM3ErUTJPNnBpei9haTI3MjdBSTc4aldpM2FzbTk5?=
 =?utf-8?B?S2RZaUxaYjQ3YnFVOXpOYWJ4NDhxV2F0R3NtM1pWaHdaek5FcU0zMXduWGoy?=
 =?utf-8?B?L1RFQ2FOMHlBNFFSWjFmV2VrVnlHaERscmRIWmRSZVBxYW9NaVRMZjR4cDJB?=
 =?utf-8?B?aHA1UW0rMTNyeGhEZW9ocVluazBaM1FhR3hCWnBJVE5sYy9HQXZlejRnbWE0?=
 =?utf-8?B?WWs0L0J3eEdCNC80VGpWM1kzMTIwRDNMVm5hWkN2TE5RWnhJeEJ4WG5tdWZo?=
 =?utf-8?B?QmU2c0FMRXdwRHY2RDhlNEFKam9KWE9uVXRlZWNFK0MwaWNpS05ERFRaOHk2?=
 =?utf-8?B?SnJ1a2ozaWhDVHJoeGEyUWZ6ZXptdFd3UU9RQmN5ZFdTaXBYWEEyUW1nb3JY?=
 =?utf-8?B?MjlzY3ByZ1RZd2ZzTUlneWxnenlCdmE0YzdzRDYrSEZPb2ZPNWNWMXZ5aWdD?=
 =?utf-8?B?Z043ajgwSVpGMWU1MFcyLzViaERBT0pQOUtweGxJNjRuQTk1N1hJK0VrODV0?=
 =?utf-8?B?TmREK3RGSzRrWWpUR1NJamRpQWFkMld4a2RBWkR1RmxOK1EwN0ZCMmM4RnMx?=
 =?utf-8?B?L3VaU3JyK1pqQU1vTkd6bDd3a09wVDF6L29VYzlXK0ZMSVd5MkFiQnJKVDBh?=
 =?utf-8?B?Q3AyNFkzUUZQWWhlSGhURjduSTdsakxTdVdrcmJsdk9SazhpOTFjTlhmYlBY?=
 =?utf-8?B?VFNBUVF6RGZrclFFUU1LYWxUdU9lbUtZOVl4Vllnb0dZNDRFMzFBcW5KTVdj?=
 =?utf-8?B?N1o5U3BJelBOZkhsNXdieENNK0VUTDFoaUx5K3FFRXkzdW5MOGlyWE01UUcr?=
 =?utf-8?B?cldYdjBjQ2FrVjZrTWtCeVQ3dUlTdGJNNG1WbW1BNnFSUjlNNnV2YW43Tjk4?=
 =?utf-8?B?ZUM5UDladUZ5VXVoQVFyTjVGeHlmZ0xOS3JNMWdoTXJZUUZJZFpMMU93ZFZW?=
 =?utf-8?B?UC9ycUZKcEJxR3orZXdtSGZycXpQSnVkdGdMc01tWmxHbkNGaDZLQmdqSzF3?=
 =?utf-8?B?UDNiZUdsekxkRkNITXd6eUdoWHg4OUg3UXV0RmlKY3lHZkhYYzNtMXJwRGZY?=
 =?utf-8?B?TWE3b1p3bXpZZnZOa2dna3IrS3Z5N29wWkhQdjJBYW4ybTd2Z1ZkdjdVaERQ?=
 =?utf-8?B?UnIwazRib3BQUy9Ob3VxSk9LeDZCSjNOb1B3c3JBMWR6ZnRpVWwzRWNic2pR?=
 =?utf-8?B?RWxqVVVINERCYXZsbXNhOUx1TnNETXJuL3hYMEEzd1FndE56MUVabGk5Z2Vq?=
 =?utf-8?B?dFQ5c0RqaGZUc1RlYU4wcjIxa0hNbkwxWlU5Q3NqWERCZC9DZFRUQkZkNXVQ?=
 =?utf-8?B?eEtUUGVUMXNPM29pREtKYk45TzhkeEV2dGdPL1pveDBUVld4elA0YTJWN3g2?=
 =?utf-8?B?WS9Ob205Ly9Fa1VMUUxodVo2eHp0MEowb0hmdzYxbW1XZ21NM0lLQkV1UXJo?=
 =?utf-8?B?S1BYclNXVFdxUFd2TVpXNnFXMnF3T3FXYmJaMDBoNlZmWnJlMzQ1cERhRmxO?=
 =?utf-8?B?V1NUSVNaK3lrNHZyUEZSWk9KdE9vZ2ZNV20xWWFpVGVHMDRhNVBYRjMwUVI5?=
 =?utf-8?B?aXNyclNLbU5qYitiSnVteXp2ekVERkdva1ZKSDRhTVZ1Y1NJZ0lBOUJwS1lJ?=
 =?utf-8?B?QUJEU05KQjRkbTY0dXQ3YUtadndpb0VMcVNJd3dFRzVlWFNiOVozRUtQbU5D?=
 =?utf-8?B?clhXam1Eb3NuV0o3OWJVOVZMZUJhajMxRUhFSzM0bE95MmdVVVo1Y3E3VzRv?=
 =?utf-8?Q?RkpAw/qSC4OEtpzcIeu9bvg=3D?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8971e5f7-2f40-4d8e-8434-08da0272b589
X-MS-Exchange-CrossTenant-AuthSource: HK2PR06MB3492.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Mar 2022 08:48:12.3437
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2G9gkKX+fjlgAkdwAtUC+aRbpnut/JAcnDAGqIzR9nPuuTpHal5yFtAZUgTZFt9cPqrE/g8mx1YxnIkMgfAq3Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY2PR06MB2895
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2022/3/10 16:27, Marc Zyngier wrote:
> On Thu, 10 Mar 2022 05:02:38 +0000,
> Guo Zhengkui <guozhengkui@vivo.com> wrote:
>>
>> `ic->fiq_aff[fiq]` is a pointer to the unnamed struct.
>> `sizeof(ic->fiq_aff[fiq])` intends to get the size of this pointer. But
>> readers may get confused. `sizeof(unsigned long)` makes more sense because
>> `unsigned long` has the same size of pointer.
> 
> More sense? It really depends on who reads the code.
> 
>>
>> reference:
>> https://lore.kernel.org/all/Ya%2FeUbdN1+ABFVWf@rowland.harvard.edu/
>> https://lore.kernel.org/all/YbBGGI9wQenI4kP7@kroah.com/
>> https://lore.kernel.org/all/20211209062441.9856-1-guozhengkui@vivo.com/
>>
>> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
>> ---
>>   drivers/irqchip/irq-apple-aic.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
>> index b40199c6625e..23098b469b1a 100644
>> --- a/drivers/irqchip/irq-apple-aic.c
>> +++ b/drivers/irqchip/irq-apple-aic.c
>> @@ -810,7 +810,7 @@ static void build_fiq_affinity(struct aic_irq_chip *ic, struct device_node *aff)
>>   	if (WARN_ON(n < 0))
>>   		return;
>>   
>> -	ic->fiq_aff[fiq] = kzalloc(sizeof(ic->fiq_aff[fiq]), GFP_KERNEL);
>> +	ic->fiq_aff[fiq] = kzalloc(sizeof(unsigned long), GFP_KERNEL);
> 
> And by doing that, you are making even more difficult to spot the
> glaring bug that is in front of your eyes: we're not trying to
> allocate a pointer, but to allocate the full data structure.
> 

Oh, I surely made a big mistake...

> Nothing went wrong as a 64bit allocation is plenty when you need at
> most 10 bits, but jeez, what a howler. I'm stashing the fixlet below
> on top.
> 

So, will you send this new patch by yourself?

Zhengkui

> Thanks,
> 
> 	M.
> 
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index b40199c6625e..3f1d2f3ccb7f 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -810,7 +810,7 @@ static void build_fiq_affinity(struct aic_irq_chip *ic, struct device_node *aff)
>   	if (WARN_ON(n < 0))
>   		return;
>   
> -	ic->fiq_aff[fiq] = kzalloc(sizeof(ic->fiq_aff[fiq]), GFP_KERNEL);
> +	ic->fiq_aff[fiq] = kzalloc(sizeof(*ic->fiq_aff[fiq]), GFP_KERNEL);
>   	if (!ic->fiq_aff[fiq])
>   		return;
>   
>