On 07/04/2018 06:18 PM, Joe Hershberger wrote:
On Wed, Jul 4, 2018 at 4:25 AM, Alexander Graf <ag...@suse.de> wrote:
On 07/04/2018 02:36 AM, Joe Hershberger wrote:
Rather than crashing, check the src ptr and set dst to empty string.

Signed-off-by: Joe Hershberger <joe.hershber...@ni.com>

Wouldn't it make more sense to check for the existence outside at the
caller's side? That way it's much easier to see what really is happening.
It's much easier to allow NULL so that we can directly pass the return
result of getenv().

I know, and I see how it looks insanely smart and simple today. Until you realize that the amazing "copy_filename" function doesn't touch the target at all if it gets passed in NULL. And all of that implicitly. So implicitly it will leave the old value in the filename if nothing is set in env.

Imaging you're trying to read the code 4 years from now. Will you remember all of the side effects of copy_filename()? Imagine you're someone who's new to the code and doesn't know all the implicit side effects of its functions. Will you understand what is going on at a glimpse?


Alex

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to