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 v2][next] xfs: Replace one-element arrays with
 flexible-array members
To:     "Darrick J. Wong" <djwong@kernel.org>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc:     Dave Chinner <david@fromorbit.com>, linux-xfs@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
References: <20210310020108.GA279881@embeddedor>
 <20210311031745.GT3419940@magnolia>
From:   "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Message-ID: <b9e2c900-0852-fc5d-9696-835636b4219f@embeddedor.com>
Date:   Wed, 10 Mar 2021 22:28:22 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210311031745.GT3419940@magnolia>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 3/10/21 21:17, Darrick J. Wong wrote:

> Why not use size_t instead of uint?  You converted the @len declaration
> above.
> 
>> +				 src_efi_fmt->efi_nextents);
>> +	uint len64 = struct_size((xfs_efi_log_format_64_t *)0, efi_extents,
> 
> Also, please don't use the struct typedefs, we're trying to get rid of
> those slowly.
> 
> TBH I wonder if these could just be turned into static inline helpers to
> decrapify the code:
> 
> static inline size_t
> sizeof_efi_log_format32(unsigned int nr)
> {
> 	return struct_size((xfs_efi_log_format_32_t *)0, efi_extents, nr);
> }
> 
> Then you only need:
> 
> 	size_t len = sizeof_efi_log_format(src_efi_fmt->efi_nextents);
> 	size_t len32 = sizeof_efi_log_format32(src_efi_fmt->efi_nextents);
> 	size_t len64 = sizeof_efi_log_format64(src_efi_fmt->efi_nextents);
> 
> 	if (len == len32) ...
> 	else if (len == len64) ...
> 
> And down below you can clean up the asserts a bit:
> 
> 	ASSERT(item->ri_buf[0].i_len ==
> 			sizeof_efi_log_format32(efd_formatp->efd_nextents) ||
> 	       item->ri_buf[0].i_len ==
> 			sizeof_efi_log_format64(efd_formatp->efd_nextents));
> 

Done: https://lore.kernel.org/lkml/20210311042302.GA137676@embeddedor/

Thanks for the feedback!
--
Gustavo