9 Feb
2019
9 Feb
'19
6:41 p.m.
Hi Fat-Zer,
although it is clear that Russell's patch will not be able to solve all of
the problems mentioned in 1766, it is a good step forward - quickly and
simply solving one particular problem. Therefore, there is good reason to
merge it.
Cheers
--
Slávek
On Friday 08 of February 2019 00:10:24 Fat-Zer wrote:
Hi,
Note that this is essentially a lighter version of the issue in bug
1766.
I'm digging it right now, but a simple adding of the DISPLAY won't fix
it for me because as for now there are actually 2 instances for each
of two screens (4 in total, see the bug for details) are fighting over
control. Having a squared-number-of-screen kdesktop_lock running all
the time just looks messed up. Also they conflict with each other when
locking a desktop from a «wrong screen».
So, I'd rather get rid of all of those constantly running
kdesktop_lock(s) and leave only one or one-for-each-screen at most.
But it requires some thine and gentle tweaking to achieve it and not
compromise the security/stability.
I've tried to rearrange things in the `main()` at first (to lock and
wait for a signal from kdesktop before forking), but it didn't gone
specifically well, actually I've got caught up into some sort of
cabbage-goat-wolf puzzle but with a flesh-eating cabbage this time...
As for now I'm looking forward for some changes in
kdesktop/kdesktop_lock or inter-kdesktop communications. Also I was
going to commit some refactoring on the kdesktop/lock/main.cc because
as for now that mess is unmodifiable (on rather high level)...
чт, 7 февр. 2019 г. в 22:41, Slávek Banko <slavek.banko(a)axis.cz>cz>:
On Thursday 07 of February 2019 16:52:21 Russell
Brown wrote:
Hi,
I don't know how closely the devs watch the Bug list but I believe
I've fixed Bug 2998.
As this bug results in kdesktop/kdesktop_lock eating CPU and going
unresponsive; might I humbly request that the fix is incorporated
in the next possible/dev version.
As the code I posted on bugzilla used a crude fixed buffer, here's
a cleaned up version that uses malloc.
*** /tmp/tdebase-trinity-14.0.6~pre38/kdesktop/lock/main.cc Thu
Feb 7 15:48:21 2019 ---
/usr/tmp/tdebase-trinity-14.0.6~pre38/kdesktop/lock/main.cc Sun May
20 19:41:55 2018 ***************
*** 325,336 ****
#endif
}
! char *locknameroot="kdesktop_lock_lockfile.";
! char *lockfilename = (char*)malloc(strlen(locknameroot) +
strlen(getenv("DISPLAY")) + 1); !
strcpy(lockfilename,locknameroot);
! strcat(lockfilename,getenv("DISPLAY"));
!
! TDELockFile lock(locateLocal("tmp", lockfilename));
lock.setStaleTime(0);
TDELockFile::LockResult lockRet = lock.lock();
if (lockRet != TDELockFile::LockOK) {
--- 325,331 ----
#endif
}
! TDELockFile lock(locateLocal("tmp",
"kdesktop_lock_lockfile")); lock.setStaleTime(0);
TDELockFile::LockResult lockRet = lock.lock();
if (lockRet != TDELockFile::LockOK) {
Hi Russell,
your observation gives a very clear sense. The name of the lock does
not distinguish between $DISPLAY and therefore two separate
kdesktop_lock processes struggles for one file. Good conclusion!
Because locateLocal expects a TQString value as argument, I suggest
using this fact and make the patch simpler:
--- a/kdesktop/lock/main.cc
+++ b/kdesktop/lock/main.cc
@@ -325,7 +325,7 @@
#endif
}
- TDELockFile lock(locateLocal("tmp",
"kdesktop_lock_lockfile")); + TDELockFile
lock(locateLocal("tmp",
TQString("kdesktop_lock_lockfile.%1").arg(getenv("DISPLAY"))));
lock.setStaleTime(0);
TDELockFile::LockResult lockRet = lock.lock();
if (lockRet != TDELockFile::LockOK) {
If you want to get involved, the best way is to register on TDE Gitea
Workspace and create a pull-request. See:
https://wiki.trinitydesktop.org/TDE_Gitea_Workspace
Note: Pull-requests should always be on the master branch. Core
developers then take care of a backport patch into a stable branch.
Note1: By the way, in your proposed code is missing free, so this
would leads to memory leaks.
Cheers
--
Slávek