On 8/10/2023 5:52 PM, Bryan O'Donoghue wrote:
On 10/08/2023 07:11, Komal Bajaj wrote:
+ if (!cfgs || cfgs->num_config != DEF_NUM_CFG) {
+ ret = -EINVAL;
+ goto err;
+ }
+ cfg = &cfgs->llcc_config[DEF_NUM_CFG - 1];
This is a bit of a redundant check.
You add in the check for num_config != 1, then deref llc_config[0] but
in patch #4 you get an index and check that index against num_config
I would take this blame on me to suggest this..,but i was trying to
avoid the hard-coding initially done for [1], now, num_config[2]
converted to ARRAY_SIZE(), i find no harm in checking
cfgs->num_config > DEF_NUM_CFG
since, anyways it will move to different function in #4.
[1]
https://lore.kernel.org/lkml/39b4bafd-410f-cae8-13ae-e18d751b51a2@xxxxxxxxxxx/
[2]
.num_cfgs = 1,
-Mukesh
I'm not seeing how at this point in your series, how num_config could be
anything other than 1.
I'd do away with the DEF_NUM_CFG define in this code/series completely.
num_config should encode all the necessary detail we need, DEF_NUM_CFG
just adds noise.
---
bod