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. -- David Marchand
