Re: [PATCH 1/6] ASoC: Intel: catpt: Fix offset checks

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2025-11-24 8:06 PM, Andy Shevchenko wrote:
> On Mon, Nov 24, 2025 at 09:05:06PM +0200, Andy Shevchenko wrote:
>> On Mon, Nov 24, 2025 at 07:20:48PM +0100, Cezary Rojewski wrote:
>>> On 2025-11-24 3:31 PM, Andy Shevchenko wrote:
>>>> On Fri, Nov 21, 2025 at 12:43:26PM +0100, Cezary Rojewski wrote:
> 
> ...
> 
>>>>> static int catpt_restore_memdumps(struct catpt_dev *cdev, struct dma_chan *chan)
>>>>
>>>>>    		off = catpt_to_host_offset(info->offset);
>>>>> -		if (off < cdev->dram.start || off > cdev->dram.end)
>>>>> +		if (off < cdev->dram.start || off + info->size >= cdev->dram.end)
>>>>>    			continue;
>>>>
>>>> Hmm... I would rather do something like
> 
>>>> 		struct resource r;
> 
> You might need to nullify this as well:
> 
> 		struct resource r = {}; // or any equivalent

This made me realize variables 'r1', 'r2' and 'common' that are part of 
catpt_restore_fwimage() are not zeroed and utilizing resource_xxx() API 
without doing so is bogus. My plan is to the following: leave this very 
fix as-is (without refactoring) and follow it up immediately with patch 
that swaps manual manipulation with resource_xxx(), as you suggested.

Sounds good?

>>>> 		...
>>>> 		resource_set_range(catpt_to_host_offset(info->offset), info->size);
>>>> 		if (!resource_contains())
>>>> 			continue;
>>>>
>>>>>    		dev_dbg(cdev->dev, "restoring memdump: off 0x%08x size %d\n",
>>>>
>>>> OTOH it seems more invasive change for kinda a fix.
>>>
>>> Looks elegant though. My idea was to avoid additional operations (which
>>> resource_contains() does), perhaps unnecessarily as readability suffered
>>> because of that. Ack.
>>
>> Note, it's defined as static inline, if compiler can prove constiness of
>> something, it will eliminate the code. TL;DR: let compiler to do its job.
>