Attention is currently required from: Light. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62724 )
Change subject: pony_spi.c: Fixed memory leak in function pony_init_spi ......................................................................
Patch Set 1:
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62724/comment/9628eccf_f3b982d7 PS1, Line 9: Memory leaked was caused as data variable wasn't deallocated in some error cases where : the function returned without deallocatiing it. Please wrap commit message lines at 72 characters, c.f. https://flashrom.org/Development_Guidelines#Commit_message
File pony_spi.c:
https://review.coreboot.org/c/flashrom/+/62724/comment/8a9dfab8_dbaf898a PS1, Line 164: free(data); We use tabs for indentation, and a tab is 8 characters wide. Please refer to https://flashrom.org/Development_Guidelines#Coding_style
Besides the indentation, this is correct. The shutdown function hasn't been registered (see comment on line 167), so `data` leaks.
https://review.coreboot.org/c/flashrom/+/62724/comment/7260c2c7_1e1d0a77 PS1, Line 167: Once execution reaches this point, we know that `register_shutdown(pony_spi_shutdown, data)` has been called successfully. This means flashrom will automatically execute the `pony_spi_shutdown()` function before exiting, which in turn calls `free(data)`. So, there's no need to `free(data)` from this point onwards.
https://review.coreboot.org/c/flashrom/+/62724/comment/b28a00e2_9407e9d0 PS1, Line 182: free(data); This isn't needed because the shutdown function has been registered, which will run `free(data)` already. Adding `free(data)` here would result in a double-free bug.
https://review.coreboot.org/c/flashrom/+/62724/comment/63515572_6b943e63 PS1, Line 248: free(data); This isn't needed either, see my comment on line 182.
https://review.coreboot.org/c/flashrom/+/62724/comment/c1f1bcca_174a24a2 PS1, Line 255: free(data); This won't work. The `register_spi_bitbang_master()` function stores a copy of the `data` pointer and uses it as argument for the `spi_data` parameter of the functions referenced by the `bitbang_spi_master_pony` struct. The memory is released by the `pony_spi_shutdown()` function.