On Mon, Sep 21, 2020 at 06:13:59PM +0000, Rojewski, Cezary wrote:
On 2020-09-21 2:59 PM, Andy Shevchenko wrote:
On Mon, Sep 21, 2020 at 01:54:13PM +0200, Cezary Rojewski wrote:
...
- for (i = j = 0; i < FW_INFO_SIZE_MAX; i++)
if (cdev->ipc.config.fw_info[i] == ' ')if (++j == 4)break;
- for (j = ++i; j < FW_INFO_SIZE_MAX && j - i < 20; j++) {
This should have static_assert() at the place where you define both constants (2nd is mentioned above 20).
if (cdev->ipc.config.fw_info[j] == ' ')break;*(pos + j - i) = cdev->ipc.config.fw_info[j];- }
- pos += 20;
These two for-loops should have some comment to explain what's going on.
Actually, after poking my FW friends again I realized that it's just dumping 20chars from "hash" segment of fw_info (struct catpt_fw_ready, field: fw_info[]).
So, this could be replaced by:
/* navigate to fifth info segment (fw hash) */ for (i = j = 0; i < FW_INFO_SIZE_MAX; i++) /* info segments are separated by space each */ if (cdev->ipc.config.fw_info[i] == ' ') if (++j == 4) break;
...and this is repeating strnchr() / strnchrnul().
With the questions "what if...": - nul in the middle of this? - less than 4 spaces found?
memcpy(pos, &cdev->ipc.config.fw_info[++i], CATPT_DUMP_HASH_SIZE); pos += CATPT_DUMP_HASH_SIZE;
Existing for-loops were based on internal solution. Half of the code isn't needed afterall..