davidedmundson added a comment.
Not to spam this review request:
> We're still looking for someone to take the time to properly benchmark
that, the last time this happened was 10 years ago at least.
FWIW: I did a very very basic test recently.
I put Q_BENCHMARK round QProcess, and
dfaure added a comment.
... because this isn't kde4 anymore, where almost every app provided a
kdeinit module.
QProcess forks from the process that uses QProcess, it might look
inconsistent when looking at process trees but it's really the most standard
Unix way of executing processes:
drosca added a comment.
Though on the other hand, why isn't KLauncher (KRun) used for running apps
everywhere? This way they are forked from `kdeinit5` and this issue is
non-existant.
Quick test:
- Launch from taskmanager -> forked from plasmashell
- Launch from quicklaunch
bshah added a comment.
In https://phabricator.kde.org/D8806#167510, @drosca wrote:
> Why? It's not like any other process can't open /dev/rfkill as we ship udev
rule to enable r+w access to /dev/rfkill for all users.
My comment was question :-)
But well I didn't know about
drosca added a comment.
In https://phabricator.kde.org/D8806#167509, @bshah wrote:
> I do wonder if this is security fix? And should probably be handled like
that?
Why? It's not like any other process can't open /dev/rfkill as we ship udev
rule to enable r+w access to
bshah added a comment.
I do wonder if this is security fix? And should probably be handled like that?
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To: ofreyermuth, davidedmundson
Cc: drosca, bshah, broulik, #frameworks
This revision was automatically updated to reflect the committed changes.
Closed by commit R269:ae044d6dfec9: Do not leak rfkill file descriptors.
(authored by ofreyermuth, committed by drosca).
REPOSITORY
R269 BluezQt
CHANGES SINCE LAST UPDATE
ofreyermuth added a comment.
In https://phabricator.kde.org/D8806#167503, @drosca wrote:
> I need your full name and e-mail to commit it (I can't get it with `arch
patch` probably because you didn't upload this review with `arc diff`).
Oliver Freyermuth
drosca added a comment.
Alright, thanks for the explanation.
I need your full name and e-mail to commit it (I can't get it with `arch
patch` probably because you didn't upload this review with `arc diff`).
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
ofreyermuth added a comment.
In https://phabricator.kde.org/D8806#167483, @drosca wrote:
> Just for the record, how does Konsole inherit this fd when BluezQt is only
used in plasmashell + kded and KDE apps are afaik not forked from these
processes?
What exactly happens depends
ofreyermuth added a comment.
In https://phabricator.kde.org/D8806#167457, @bshah wrote:
> Do you have commit access?
In case you are adressing me: No.
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To: ofreyermuth, davidedmundson
Cc: drosca,
drosca added a comment.
Just for the record, how does Konsole inherit this fd when BluezQt is only
used in plasmashell + kded and KDE apps are afaik not forked from these
processes?
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To: ofreyermuth,
bshah added a comment.
Do you have commit access?
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To: ofreyermuth, davidedmundson
Cc: bshah, broulik, #frameworks
ofreyermuth added a comment.
In https://phabricator.kde.org/D8806#167404, @broulik wrote:
> If you add the following (upper-case with a colon and space) on its own
line in your commit message [...]
Many thanks, done!
REPOSITORY
R269 BluezQt
REVISION DETAIL
ofreyermuth edited the summary of this revision.
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To: ofreyermuth, davidedmundson
Cc: broulik, #frameworks
broulik added a comment.
If you add the following (upper-case with a colon and space) on its own line
in your commit message the bug will be automatically closed once it landed:
BUG: 386886
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To:
ofreyermuth added a comment.
Thanks for the quick review!
I also created a related issue report here:
https://bugs.kde.org/show_bug.cgi?id=386886
which can of course be closed once the patch has landed.
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To:
davidedmundson accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R269 BluezQt
REVISION DETAIL
https://phabricator.kde.org/D8806
To: ofreyermuth, davidedmundson
Cc: #frameworks
ofreyermuth created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
REVISION SUMMARY
They may be leaked into all child processes,
including regular Konsole terminals on KDE.
I observed them showing up at all
19 matches
Mail list logo
