Hi Simon,
On Wed, Apr 17, 2019 at 09:33:06PM -0700, Simon Glass wrote:
Hi Eugeniu,
On Mon, 1 Apr 2019 at 03:46, Eugeniu Rosca erosca@de.adit-jv.com wrote:
Paranoid programming [1] lies at the foundation of proper software development, but the repetitive zeroing-out of output arguments in the context of the same function rather clutters the code and inhibits further refactoring/optimization than is doing any good.
In boot_get_fdt(), we already perform zero/NULL-initialization of *of_flat_tree and *of_size at the beginning of the function, so doing the same at function error-out is redundant/superfluous.
Moreover, keeping the code unchanged might encourage the developers to update *of_flat_tree and *of_size during some interim computations, which is against the current design of boot_get_fdt(). Currently, writing useful data into these arguments happens just before successfully returning from boot_get_fdt() and it should better stay so.
[1] https://blog.regehr.org/archives/1106
Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com
Changes in v2:
- s/zeroint-out/zeroing-out/ in commit description
- Link v1: https://patchwork.ozlabs.org/patch/1071586/
common/image-fdt.c | 2 -- 1 file changed, 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
But please update the comment to for the function:
of_flat_tree and of_size are set to 0 if no fdt exists
Thank you very much for the review. Since the patch is part of a series and there are no other comments except this one, should I decouple it and send as v3 standalone or there is still some chance for getting feedback for the other patches (and sending an update for the whole series)?
Regards, Simon
Best regards, Eugeniu.