Missed the part about moving to rpmio/rpmlua.c... anyway, been long enough - I
adjusted that part myself and pushed as commit
18e792340e33eade01967d01d5801f1ae9e0ad33 (and
cb2ae4bdf2f60876fdc68e3f84938e9c37182fab).
Thanks for the patches!
--
You are receiving this because you are subscribed
Closed #390.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/390#event-1661014629___
Rpm-maint mailing list
I've rebased this and added use of the function from #444. Please review.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Once #444 lands, this should be updated to use it and move the code to
rpmio/rpmlua.c and then it'd probably be good to go.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@ignatenkobrain , that's missing the point entirely. It's all the already open
descriptors that we need to take care of, including whatever that might be
opened by dnf and all the other API consumers.
--
You are receiving this because you are subscribed to this thread.
Reply to this email
@pmatilai I don't think that I open file descriptors by hand in this function,
so there is nothing to close..
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
The real flaw with looping over close(fdno) before exec in rpm is that it masks
the root cause of the problem: applications that are not setting FD_CLOEXEC.
The loop also prevents an application from intentionally passing an open fdno
to a child of rpm (if that is/was intended).
I'd suggest
BTW, wrt closing file descriptors, we can't go adding copy-paste versions of
that all over the place, two places is one too many already. One possibility
might be having a central posix_atfork() handler to take care of it.
Central handling is also necessary to do something about
Don't consider this an actual review, but two things that occurred to me:
- this needs to close open file descriptors (see eg commits
7a7c31f551ff167f8718aea6d5048f6288d60205 and
aa9a791d808f504781d0b75255df3387383a1809)
- technically this belongs to rpmio/ side because it does not depend on
@pmatilai in any case, any rebase / changes in posix module in rpm/lua doesn't
help here. I just want function which executes process without using shell
which would be very simple.. So this is what this PR is about. Could you review
it please? ;)
--
You are receiving this because you are
Yes there are a couple of hacks to lposix which means we can't just switch to
upstream, rebasing to newer one is an entirely another thing, and was under
discussion last year already (there's a ticket around here somewhere).
The other thing is that there are other ways around eg the exec()
I wonder if @n3npq's idea of rebasing the luaposix we have included in RPM is a
better idea... Both the `posix` and `rexlib` modules were copied into the
source tree a long time ago, maybe rebasing is a better solution?
@pmatilai what do you think?
--
You are receiving this because you are
@Conan-Kudo no, they don't use malloc, they use `lua_newuserdata` which is
managed by Lua itself IIUC.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@ignatenkobrain Is the memory leak fix needed in upstream
[luaposix](https://github.com/luaposix/luaposix)?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Alright, moved under RPM namespace and renamed to `execute()` which better
aligns with what function really does.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
15 matches
Mail list logo
