RE: [PATCH v7 03/14] ASoC: Intel: catpt: Add IPC message handlers

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

On 2020-09-22 11:04 AM, Andy Shevchenko wrote:
> On Mon, Sep 21, 2020 at 08:48:12PM +0000, Rojewski, Cezary wrote:
>> On 2020-09-21 8:41 PM, Andy Shevchenko wrote:

...

>> While this should never happen (means user is somehow not making use of
>> officially released firmware binary), coredumps are useful only if you
>> have access to debug tools. In cases you'd mentioned, invalid hash would
>> have been dumped to coredump and crash reader simply wouldn't have been
>> able to navigate to actual build for it. The rest of the coredump is still
>> vital though.
>>
>> memcpy() could be gated behind an 'if' for safety if needed:
>>
>> 	info = cdev->ipc.config.fw_info;
>> 	eof = info + FW_INFO_SIZE_MAX;
>> 	/* navigate to fifth info segment (fw hash) */
>> 	for (i = 0; i < 4 && info < eof; i++, info++)
>> 		/* info segments are separated by space each */
>> 		if ((info = strnchr(info, eof - info, ' ')) == NULL)
>> 			break;
> 
>> 	if (i == 4 && info < eof)
>> 		memcpy(pos, info, min(eof - info, CATPT_DUMP_HASH_SIZE));
> 
> And here basically enough check is info against NULL, right?
> Just try to look at different possibilities how to make code simpler and neater.
> 
>> Didn't compile this, some typecheck fixes might be in order and so on.
> 

What you meant is:
	if (i == 4 && !info) // instead of 'info < eof'

right?

If 4th space is last char in this string then info would end up being
non-NULL and equal to 'eof' and thus memcpy() would get invoked with
size=eof-info=0.

catpt_coredump() is here to gather debug info for Intel folks to analyze
in case of critical error. In ideal world, it should not be required at
all as when we get here, there are bigger problems on our head.
Above solution is simpler than what is prevent in v7 while also
maintaining good readability - variable names - plus comments which you
suggested.

Czarek