> It's perfectly legal to call kfree() on a NULL pointer. I know this function property well. >> * Split a condition check for memory allocation failures so that >> each pointer from these function calls will be checked immediately. >> >> See also background information: >> Topic "CWE-754: Improper check for unusual or exceptional conditions" >> Link: https://cwe.mitre.org/data/definitions/754.html >> >> * Return directly after a call of the function "kzalloc" failed >> at the beginning. > > Both calls are already close together. Can it be that an other software development concern is eventually overlooked because of this "neighbourship" (or is categorised with a lower priority)? I suggest to reconsider this design detail if it is really acceptable for the safe implementation of such a software module. * How much will it matter in general that one function call was performed in this use case without checking its return values immediately? * Should it usually be determined quicker if a required resource like memory could be acquired before trying the next allocation? > In addition, your patch increases the LoC, IMHO without improving the code. I find this consequence still debatable. >> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c >> index 1fdc44b..6c82e0e 100644 >> --- a/drivers/clk/renesas/clk-mstp.c >> +++ b/drivers/clk/renesas/clk-mstp.c >> @@ -167,10 +167,12 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) >> unsigned int i; >> >> group = kzalloc(sizeof(*group), GFP_KERNEL); >> + if (!group) >> + return; >> + >> clks = kmalloc_array(MSTP_MAX_CLOCKS, sizeof(*clks), GFP_KERNEL); >> - if (group == NULL || clks == NULL) { >> + if (!clks) { >> kfree(group); >> - kfree(clks); >> return; >> } Is this update suggestion worth for another look? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
