tag #522 pending fixed #522 4.0.2.1 thanks Hello, X2Go issue #522 (src:x2goclient) reported by you has been fixed in X2Go Git. You can see the changelog below, and you can check the diff of the fix at: http://code.x2go.org/gitweb?p=x2goclient.git;a=commitdiff;h=093543e The issue will most likely be fixed in src:x2goclient (4.0.2.1). light+love X2Go Git Admin (on behalf of the sender of this mail) --- commit 093543e62d00d8b0dcfb36798fa48bd8757fb17e Author: Oleksandr Shneyder <o.shneyder@phoca-gmbh.de> Date: Fri Jun 27 10:33:43 2014 +0200 Fix running x2goclient without arguments on Windows. (Fixes: #522). diff --git a/debian/changelog b/debian/changelog index 349f8ea..e756295 100644 --- a/debian/changelog +++ b/debian/changelog @@ -30,6 +30,7 @@ x2goclient (4.0.2.1-0x2go1) UNRELEASED; urgency=low - Check if sound is activated before starting pulse. - Fix starting sshd on Win XP. (Fixes: #421). - Fork x2goclient on windows and terminate child processes if x2go client crashed. + - Fix running x2goclient without arguments on Windows. (Fixes: #522). - Save proxy output in $HOME/S-$SESSION-ID/session.log if debugging is enabled. [ Mike DePaulo ] * New upstream release (4.0.2.1):
Processing commands for control@bugs.x2go.org:
tag #522 pending Bug #522 [x2goclient] X2Go Client fails to start if started with no cmdline arg Added tag(s) pending. fixed #522 4.0.2.1 Bug #522 [x2goclient] X2Go Client fails to start if started with no cmdline arg There is no source info for the package 'x2goclient' at version '4.0.2.1' with architecture '' Unable to make a source version for version '4.0.2.1' Marked as fixed in versions 4.0.2.1. thanks Stopping processing here.
Please contact me if you need assistance.
522: http://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=522 X2Go Bug Tracking System Contact owner@bugs.x2go.org with problems
Alex, Thank you for fixing this quickly. I can confirm it is fixed in the nightly build: http://code.x2go.org/releases/binary-win32/x2goclient/heuler/mingw32-4.4/qt-... However, Ionic and I were taking a look at the code last night, and we did not understand why "--child-process" is being added to the args list. It looks like you are adding it when it does not exist in the list. If so, why not just hardcode the behavior that --child-process invokes? -Mike On Fri, Jun 27, 2014 at 4:33 AM, Oleksandr Shneyder <o.shneyder@phoca-gmbh.de> wrote:
tag #522 pending fixed #522 4.0.2.1 thanks
Hello,
X2Go issue #522 (src:x2goclient) reported by you has been fixed in X2Go Git. You can see the changelog below, and you can check the diff of the fix at:
http://code.x2go.org/gitweb?p=x2goclient.git;a=commitdiff;h=093543eThe issue will most likely be fixed in src:x2goclient (4.0.2.1).
light+love X2Go Git Admin (on behalf of the sender of this mail)
commit 093543e62d00d8b0dcfb36798fa48bd8757fb17e Author: Oleksandr Shneyder <o.shneyder@phoca-gmbh.de> Date: Fri Jun 27 10:33:43 2014 +0200
Fix running x2goclient without arguments on Windows. (Fixes: #522).diff --git a/debian/changelog b/debian/changelog index 349f8ea..e756295 100644 --- a/debian/changelog +++ b/debian/changelog @@ -30,6 +30,7 @@ x2goclient (4.0.2.1-0x2go1) UNRELEASED; urgency=low - Check if sound is activated before starting pulse. - Fix starting sshd on Win XP. (Fixes: #421). - Fork x2goclient on windows and terminate child processes if x2go client crashed.
- Fix running x2goclient without arguments on Windows. (Fixes: #522).
- Save proxy output in $HOME/S-$SESSION-ID/session.log if debugging is enabled. [ Mike DePaulo ]
- New upstream release (4.0.2.1):
I would so reject this patch. It's logically flawed.
First: 106 QStringList args; 107 if ( argc > 1 ) 108 args=app.arguments();
Later: 154 if(argc <=1) 155 { 156 args=app.arguments(); 157 }
Remove those two occurrences and use: 106 QStringList args; 107 args=app.arguments();
The baseline issue has been args not populated with app.arguments(). It wasn't uninitialized, but empty.
An empty list is basically fine, but this will crash: 161 QString executable=args[0]; 162 args.pop_front();
args[0] is not defined in that case.
Even worse: the Qt documentation for pop_front() specifies: "The list must not be empty."
Please, use proper logic and always set args to app.arguments() without this nonsensical if (argc > 1) ... if (argc <= 1) tautology in two different places.
- On 27.06.2014 03:38 pm, Michael DePaulo wrote:
However, Ionic and I were taking a look at the code last night, and we did not understand why "--child-process" is being added to the args list. It looks like you are adding it when it does not exist in the list. If so, why not just hardcode the behavior that --child-process invokes?
Having had a closer look at the code in question, adding --child-process is fine. It's only added to the forked process's argument list. You can't hardcode the behavior of --child-process, as it's not supposed to be used when you start x2goclient.exe "normally". In contrast, --child-process lets you determine whether a process is the forked child process, or rather the manually by the user started main process.
Feel free to ignore this section, Alex.
Mihai
Hello Mihai,
The list is never empty, it have at least 1. argument - it is always executable name. I'll leave my solution here, because in a case of linux or mac if x2goclient started without arguments list don't need to be initialized. Actually, I can change argc<=1 to argc ==1 but in this case it make no difference.
regards, Alex
Am 27.06.2014 16:30, schrieb Mihai Moldovan:
I would so reject this patch. It's logically flawed.
First: 106 QStringList args; 107 if ( argc > 1 ) 108 args=app.arguments();
Later: 154 if(argc <=1) 155 { 156 args=app.arguments(); 157 }
Remove those two occurrences and use: 106 QStringList args; 107 args=app.arguments();
The baseline issue has been args not populated with app.arguments(). It wasn't uninitialized, but empty.
An empty list is basically fine, but this will crash: 161 QString executable=args[0]; 162 args.pop_front();
args[0] is not defined in that case.
Even worse: the Qt documentation for pop_front() specifies: "The list must not be empty."
Please, use proper logic and always set args to app.arguments() without this nonsensical if (argc > 1) ... if (argc <= 1) tautology in two different places.
- On 27.06.2014 03:38 pm, Michael DePaulo wrote:
However, Ionic and I were taking a look at the code last night, and we did not understand why "--child-process" is being added to the args list. It looks like you are adding it when it does not exist in the list. If so, why not just hardcode the behavior that --child-process invokes?
Having had a closer look at the code in question, adding --child-process is fine. It's only added to the forked process's argument list. You can't hardcode the behavior of --child-process, as it's not supposed to be used when you start x2goclient.exe "normally". In contrast, --child-process lets you determine whether a process is the forked child process, or rather the manually by the user started main process.
Feel free to ignore this section, Alex.
Mihai
--
Oleksandr Shneyder | Email: o.shneyder@phoca-gmbh.de phoca GmbH | Tel. : 0911 - 14870374 0 Ludwig-Feuerbach-str. 18 | Fax. : 0911 - 14870374 9 D-90489 Nürnberg | Mobil: 0163 - 49 64 461
Geschäftsführung: Dipl.-Inf. Oleksandr Shneyder
Amtsgericht München | http://www.phoca-gmbh.de HRB 196 658 | http://www.x2go.org USt-IdNr.: DE281977973
Hi Alex,
- On 27.06.2014 09:36 pm, Oleksandr Shneyder wrote:
The list is never empty, it have at least 1. argument - it is always executable name.
args will be empty on OS X and Linux, if no argument has been passed to x2goclient.exe.
I'll leave my solution here, because in a case of linux or mac if x2goclient started without arguments list don't need to be initialized.
Granted, OS and Linux don't currently use args, but what if they did? I don't like different behavior based on the OS.
If you really want to differentiate, use
106 QStringList args; 107 #ifdef Q_OS_WIN 108 args=app.arguments(); 109 #else 110 if ( argc > 1 ) 111 args=app.arguments(); 112 #endif
And lose the copying in lines 154ff.
That will help keep the code readable.
I still strongly suggest not differentiating based on the OS where not necessary, like in this case.
Mihai
Hi Alex,
On So 29 Jun 2014 20:02:57 CEST, Mihai Moldovan wrote:
Hi Alex,
- On 27.06.2014 09:36 pm, Oleksandr Shneyder wrote:
The list is never empty, it have at least 1. argument - it is always executable name.
args will be empty on OS X and Linux, if no argument has been passed to x2goclient.exe.
I'll leave my solution here, because in a case of linux or mac if x2goclient started without arguments list don't need to be initialized.
Granted, OS and Linux don't currently use args, but what if they did? I don't like different behavior based on the OS.
If you really want to differentiate, use
106 QStringList args; 107 #ifdef Q_OS_WIN 108 args=app.arguments(); 109 #else 110 if ( argc > 1 ) 111 args=app.arguments(); 112 #endif
And lose the copying in lines 154ff.
That will help keep the code readable.
I still strongly suggest not differentiating based on the OS where
not necessary, like in this case.
There is one more concern about that commit to X2Go Client code that
concerns me [1].
We will attempt at building an X2Go Client version without this commit
[2] and see if the startup speed normalizes without it.
If so, I will remove that commit (and it fixup commit later) for the
upcoming X2Go Client 4.0.2.1 release and re-add them after the release
for
more introspection and improvement targetting 4.0.2.2.
Secondly, Mihai's proposal seems to make sense to me (by just having
read everything that has been written around this issue).
Greets, Mike
[1] http://bugs.x2go.org/525
[2]
http://code.x2go.org/gitweb?p=x2goclient.git;a=commitdiff;h=76777fca627f287f...
--
DAS-NETZWERKTEAM mike gabriel, herweg 7, 24357 fleckeby fon: +49 (1520) 1976 148
GnuPG Key ID 0x25771B31 mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de
freeBusy: https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xf...
Hallo Mike, this is how x2goclient know if it running as parent or child process. Am 27.06.2014 15:38, schrieb Michael DePaulo:
Alex,
Thank you for fixing this quickly. I can confirm it is fixed in the nightly build: http://code.x2go.org/releases/binary-win32/x2goclient/heuler/mingw32-4.4/qt-...
However, Ionic and I were taking a look at the code last night, and we did not understand why "--child-process" is being added to the args list. It looks like you are adding it when it does not exist in the list. If so, why not just hardcode the behavior that --child-process invokes?
-Mike
On Fri, Jun 27, 2014 at 4:33 AM, Oleksandr Shneyder <o.shneyder@phoca-gmbh.de> wrote:
tag #522 pending fixed #522 4.0.2.1 thanks
Hello,
X2Go issue #522 (src:x2goclient) reported by you has been fixed in X2Go Git. You can see the changelog below, and you can check the diff of the fix at:
http://code.x2go.org/gitweb?p=x2goclient.git;a=commitdiff;h=093543eThe issue will most likely be fixed in src:x2goclient (4.0.2.1).
light+love X2Go Git Admin (on behalf of the sender of this mail)
commit 093543e62d00d8b0dcfb36798fa48bd8757fb17e Author: Oleksandr Shneyder <o.shneyder@phoca-gmbh.de> Date: Fri Jun 27 10:33:43 2014 +0200
Fix running x2goclient without arguments on Windows. (Fixes: #522).diff --git a/debian/changelog b/debian/changelog index 349f8ea..e756295 100644 --- a/debian/changelog +++ b/debian/changelog @@ -30,6 +30,7 @@ x2goclient (4.0.2.1-0x2go1) UNRELEASED; urgency=low - Check if sound is activated before starting pulse. - Fix starting sshd on Win XP. (Fixes: #421). - Fork x2goclient on windows and terminate child processes if x2go client crashed.
- Fix running x2goclient without arguments on Windows. (Fixes: #522).
- Save proxy output in $HOME/S-$SESSION-ID/session.log if debugging is enabled. [ Mike DePaulo ]
- New upstream release (4.0.2.1):
-- ----------------------------------------------------------- Oleksandr Shneyder | Email: o.shneyder@phoca-gmbh.de phoca GmbH | Tel. : 0911 - 14870374 0 Ludwig-Feuerbach-str. 18 | Fax. : 0911 - 14870374 9 D-90489 Nürnberg | Mobil: 0163 - 49 64 461 Geschäftsführung: Dipl.-Inf. Oleksandr Shneyder Amtsgericht München | http://www.phoca-gmbh.de HRB 196 658 | http://www.x2go.org USt-IdNr.: DE281977973 -----------------------------------------------------------
-
Michael DePaulo -
Mihai Moldovan -
Mike Gabriel -
Oleksandr Shneyder -
owner＠bugs.x2go.org