Hi Eugeniu,
On Thu, 18 Apr 2019 at 03:18, Eugeniu Rosca erosca@de.adit-jv.com wrote:
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 existsThank 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)?
I don't think there are any hard conventions. You can certainly resend v3 of just that one patch. But I don't think anyone would mind if you sent v3 of the whole series.
Regards, Simon