[U-Boot] [PATCH 3/5] net: Make copy_filename() accept NULL src

[Alexander Graf]

On 07/04/2018 06:18 PM, Joe Hershberger wrote:
> On Wed, Jul 4, 2018 at 4:25 AM, Alexander Graf <agraf at 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.hershberger at 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