Hey Alex, On Thu, Jul 5, 2018 at 12:27 PM, Joe Hershberger <joe.hershber...@ni.com> wrote: > On Thu, Jul 5, 2018 at 6:49 AM, Alexander Graf <ag...@suse.de> wrote: >> 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. > > I think you are mis-reading the code. If src is NULL, it will set > dst[0] = '\0'; I think the behavior is quite reasonable.
Do you have any outstanding issues with this? >> 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? > > That's an argument for documentation... and anyway, yes, I think the > function is sensible and I would expect it to do what it does. > > -Joe _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot