From melroy89@protonmail.com  Tue May 19 22:00:51 2020
Received: (at 1469) by bugs.x2go.org; 19 May 2020 20:01:22 +0000
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on
	ymir.das-netzwerkteam.de
X-Spam-Level: 
X-Spam-Status: No, score=-1.7 required=3.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,FREEMAIL_REPLYTO_END_DIGIT,
	SPF_HELO_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no
	version=3.4.2
Received: from mail-40132.protonmail.ch (mail-40132.protonmail.ch [185.70.40.132])
	by ymir.das-netzwerkteam.de (Postfix) with ESMTPS id CB3F95DAF0
	for <1469@bugs.x2go.org>; Tue, 19 May 2020 22:00:48 +0200 (CEST)
Date: Tue, 19 May 2020 20:00:40 +0000
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;
	s=protonmail; t=1589918447;
	bh=gh09q4SKuVWuf8A8bf8/XsJ6KSCXrLRe5VW2tVp0jjw=;
	h=Date:To:From:Reply-To:Subject:In-Reply-To:References:From;
	b=P6JWhbs0K4Icxy5KOaBsrgA0vqXRLTxwh4mOOmtKhu/oUOZvzCvCATP2DaMOQxr8T
	 IJNPUXut3BN1ylT5KX4vJSNHcAXxaTI/ri842nuY5KQ2bMBt9usA15s0YCQk7BJjjq
	 UoNGkkYAG8+8m7Pfy8hxK2JvD636vC3JGEU7DwP0=
To: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>, "1469@bugs.x2go.org" <1469@bugs.x2go.org>
From: Melroy van den Berg <melroy89@protonmail.com>
Reply-To: Melroy van den Berg <melroy89@protonmail.com>
Subject: Re: [X2Go-Dev] Bug#1469: Patch!
Message-ID: <vwmmt0MpRwz20ya2I5-8qr7-aJ2JCq98JUDJJtHfzwSTnw40ZtvMJcqJNXII0hKASmilLCHLKekLnhna6OsDx11KLVYN4IFEOq4yQCv0awg=@protonmail.com>
In-Reply-To: <20200519194833.Horde.kr6XjaMhAMOcOXrvwxL5PjI@mail.das-netzwerkteam.de>
References: <vq79NxFEEt8Pcp8OgUNEMq3mjc-pTXr2Am-M5IFBFEQ3Z2QlGHllLVmhjXXGe6fRM_hIfq91T7Y419CPz62rBb9Yn0GQL6Fc2s1kdrffjlY=@protonmail.com>
 <Nf4e-RR7tv5ZjrP9b31cyDJXHDClVE5R6fFyaE0wsZNPOk928OaXhMPa1ORMSsmATLtCa2Jp8XlSZqscpcQh-P9Em3cji311xXVTm_JUrnI=@protonmail.com>
 <20200519194833.Horde.kr6XjaMhAMOcOXrvwxL5PjI@mail.das-netzwerkteam.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi,

Well.. it was not unnecessary. The code was actually unreadable, wrong inde=
ntations and inconsistent throughout the whole file.
I needed to execute this auto-formatter, before I could even begin to under=
stand the code.

In fact, I urge you to execute this on the whole archive. And keep the code=
 quality high via a CI pipeline or something.

I can understand it makes it hard to read. I also fixed other some issues l=
ike this double error message during login and all the debug statements.
What would be your approach? I think it's still wise to auto-format the cod=
e in consistent matter.

Regards,
Melroy van den Berg

=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90 Original Me=
ssage =E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90=E2=80=90
Op dinsdag, mei 19, 2020 9:48 PM, Mike Gabriel <mike.gabriel@das-netzwerkte=
am.de> schreef:

> Hi Melroy,
>
> On Di 19 Mai 2020 03:59:16 CEST, Melroy van den Berg wrote:
>
> > Hi,
> > I did some refactoring in the sshmasterconnection.cpp file. I think > i=
t can definitely use some clean-up and further splitting functions > and ev=
en into multiple files eventually.
> > I cloned the master branch of code.x2go.org/x2goclient repo.
> > The changes I applied are in the attachment of this mail (a git > patch=
 file). I think closing previous sessions and connection solved > my strang=
e SSH connection is resulting into Socket error: No such > file or director=
y".
> > Although X2Go client is calling libssh > calls (libssh version > curren=
tly in use runtime on my host is: 0.7.0).
> > For some reason this vague error message popped-up by libssh, > properl=
y a result of previous session being created and not > correctly closed. An=
d/or not closed in the right order (/missing API > calls), like a good exam=
ple:
> > ssh_channel_send_eof(channel);
> > ssh_channel_close(channel);
> > ssh_channel_free(channel);
> > And I created yet again a new YT video showing my result:
> > https://youtu.be/vmASLJq0CKM
> > (including some brief explanation what I did so far, I also ran an > fo=
rmatted on the code, because it was very very hard to read with > all the w=
rong indents)
> > I also made another video showing yet another bug I found regarding > t=
he SSH tunneling (during some heavy testing), the only solution was > to re=
start the whole docker image:
> > https://youtu.be/xEpIyo84fWc (I think this issue is unrelated, but > ma=
ybe wise to take serious as well)
> > Again see attachment for the patch (branched from master).
> > Regards,
> > Melroy van den Berg
>
> Thanks for working on X2Go Client. With your patch, there is a problem th=
ough. It is barely unreviewable, because you have so many changes (also whi=
te space changes) in one patch file.
>
> Please clone the X2Go Client Git repo, add atomic changes with good commi=
t messages. Keep white-space changes and technical changes fully separate (=
ideally: technical changes first, white-space changes last).
>
> Then export your patches using 'git format-patch <last-official-commit>..=
HEAD' and send us the set of patches instead this one bulk commit/patch.
>
> Thanks for your efforts!!!
>
> Mike
>
> -----------------------------------
>
> DAS-NETZWERKTEAM
> c\o Technik- und =C3=96kologiezentrum Eckernf=C3=B6rde
> Mike Gabriel, Marienthaler Str. 17, 24340 Eckernf=C3=B6rde
> mobile: +49 (1520) 1976 148
> landline: +49 (4351) 850 8940
>
> GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31
> mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de


