On 12/11/2019 21:34, David Marchand wrote: > On Tue, Nov 12, 2019 at 9:29 AM Krzysztof Kanas <kka...@marvell.com> wrote: >> On 19-11-08 14:45, David Marchand wrote: >>> External Email >>> >>> ---------------------------------------------------------------------- >>> On Fri, Nov 8, 2019 at 12:05 PM David Marchand >>> <david.march...@redhat.com> wrote: >>>> On Fri, Nov 8, 2019 at 11:21 AM <kka...@marvell.com> wrote: >>>>> Hi David, >>>>> >>>>> Thanks for review, hopefully this patch will addresses most of the sutff. >>>>> Rest I will address here. >>>>> >>>>>>> + const char *procdir = "/proc/self/fd/"; >>>>>> self is a Linux thing. >>>>>> This won't work on FreeBSD. >>>>> IMHO original code didn't worked on FreeBSD as well. >>>>> I have created function to adress in third patch >>>> Indeed... >>>> >>>> Well, wait. >>>> Why do we need to close those file descriptors? >>>> FreeBSD has been like this for quite some time. >>> We don't know what this is used for. >>> >>> We know it does not work on FreeBSD, but this does not seem to be a problem. >>> Introducing something more on FreeBSD is a risk with no actual benefit >>> at first sight. >> Ok. >> >>> Either we take only the first patch under a #ifdef EXEC_ENV_LINUX or >>> we leave this as is. >> I would like have that because ARM64 is main platform for me and this >> makes the test pass, and fixes the difference between the comment and >> the code. >>> Krzystof, is this a problem for you if we postpone and investigate >>> further for 20.02? >> Not a problem. I would like to have fix for EXEC_ENV_LINUX for now. >> Rest, FreeBSD case and the decisions weather this whole code should be >> deleted can be made later. > In my tests of the original code, I never saw anything but access -1 ENOENT. > Anyway, just sent a v4, your v3 patch, with just cosmetics and > commitlog updated.
ACK. Tested, the v4 patch works fine. > > -- Best Regards -- Krzysztof Kanas kka...@marvell.com
