After upgrading all packages, it seems that "krdc" is no longer able to spawn RDP sessions. Activating all debug flags using "kdebugdialog" reveals that "krdc" executes "xfreerdp" with following options: ------------------------------------------------------------------------------- -decorations /w:1255 /h:879 /kbd:0x00000407 /u:SECRETUSER /parent-window:4194338 /bpp:16 /audio-mode:2 /drive:media,/media /network:lan /cert-ignore +clipboard /port:3389 /v:SECRETSERVER ------------------------------------------------------------------------------- Executing "xfreerdp" manually with above mentioned options line does not spawn a RDP session as well and show "xfreerdp" usage output instead. Removing an option one by one reveals that "xfreerdp" does not like the "/network" parameter anymore - at least with that options combination. Patching "krdc" to exclude that: ------------------------------------------------------------------------------- --- rdpview.cpp.bak 2018-01-30 13:08:36.375470000 +0100 +++ rdpview.cpp 2018-01-30 12:53:57.551629000 +0100 @@ -310,7 +310,7 @@ break; } - arguments << "/network:" + performance; + // arguments << "/network:" + performance; // nbe: broken in FreeRDP 2.0.0r1 if (m_hostPreferences->console()) { arguments << "/admin"; ------------------------------------------------------------------------------- makes it working again...
Reassigning to kevans@ as the maintainer of freerdp. The problem isn't that krdc is calling freerdp with bad parameters, it's that freerdp 2.0.0-rc1 (current in FreeBSD ports) has bad parameter handling. FreeBSD's strtol() sets errno to EINVAL on bad input, which freerdp doesn't expect.
Created attachment 192351 [details] Patch (to add to files/) Accept strtol() returning 0 and setting errno to EINVAL, when passed the string "lan". This is different from the behavior Linux GLibC displays.
(In reply to Adriaan de Groot from comment #2) thanks for the patch. Unfortunately, it does not fix the problem. To reduce the test case, plese try following command line: ----------------------------------------------------------------------------- xfreerdp /network:lan /port:3389 /v:127.0.0.1 ----------------------------------------------------------------------------- It still throws the usage information unless you remove the "/network:lan" part...
It shows the usage information because of *other* bugs in xfreerdp, though. I mean, sure, we can just remove /network from krdc and ignore the setting the user does there, but the problems are in xfreerdp, in strtoul() or in the way errno is set. Well, mostly in the fact that strtol() sets errno in invalid cases, but doesn't *re*set it on success, while Linux GLibC seems to ignore errno entirely.
Created attachment 192358 [details] Fix command-line handling Fix handling of errno wrt. strtol() and strtoul()
(In reply to Adriaan de Groot from comment #5) thanks for the patch - that fixes the simplified test-case with the parameters "/network:lan /port:3389 /v:127.0.0.1". But the complete command line - that "krdc" wants to spawn - still fails: ----------------------------------------------------------------------------- # bad /usr/local/bin/xfreerdp -decorations /w:1255 /h:879 /kbd:0x00000407 /u:SECRETUSER /parent-window:4194338 /bpp:16 /audio-mode:2 /drive:media,/media /network:lan /cert-ignore +clipboard /port:3389 /v:SECRETSERVER ----------------------------------------------------------------------------- unless you remove the "/network:[...]" portion: ----------------------------------------------------------------------------- # good /usr/local/bin/xfreerdp -decorations /w:1255 /h:879 /kbd:0x00000407 /u:SECRETUSER /parent-window:4194338 /bpp:16 /audio-mode:2 /drive:media,/media /cert-ignore +clipboard /port:3389 /v:SECRETSERVER -----------------------------------------------------------------------------
Kyle, there's an -rc2 for FreeRDP out now, and I've filed issue #4597 upstream referring to this PR. Can you chase the parameter-handling some more?
(In reply to Adriaan de Groot from comment #7) Hi Adriaan, Thanks for the patch and help diagnosing! I'm taking a poke at this right now -- I got kind of sidetracked in trying to get things cranked out and snuck in for the upcoming release that I've seriously neglected FreeRDP. =(
Hi, So basically, your assessment is generally right but we'll need to rework the patch... a lot. This might be more of a Coccinelle job, but basically- every call to strtol, strotul, strtold that tries to check errno != 0 after the fact should be setting errno to 0 before-hand, as per POSIX recommendations [1]. Ideally the return value of these functions would always be validated before errno is consulted, but regardless- a POSIX compliant implementation will not set errno to 0 on success, so errno != 0 checks alone to signify an error are bogus without knowing that errno = 0 going into it. [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/strtol.html
Created attachment 192835 [details] svn(1) diff of net/freerdp I've created an upstream pull request here [1] after verifying that it fixes Nils' invocation to not throw usage, at least -- I would wager that it works as intended, though. I've additionally attached here a patch to update to -rc2 w/ my patch that's been submitted in PR 4598 upstream. If you could both test it, then Adriaan may commit it or I'll do so if he approves me to do so. =) Poudriere and portlint are both relatively happy with it. [1] https://github.com/FreeRDP/FreeRDP/pull/4598
(In reply to Kyle Evans from comment #10) thanks for the patch; I no more get a usage prompt, but "xfreerdp" aborts with: -------------------------------------------------------------------------- [11:48:19:615] [5109:0a016500] [INFO][com.freerdp.client.common.cmdline] - loading channelEx cliprdr Password: [11:48:21:111] [5109:0a016500] [INFO][com.freerdp.gdi] - Local framebuffer format PIXEL_FORMAT_BGRX32 [11:48:21:111] [5109:0a016500] [INFO][com.freerdp.gdi] - Remote framebuffer format PIXEL_FORMAT_RGB16 [11:48:21:120] [5109:0a016500] [INFO][com.winpr.clipboard] - initialized POSIX local file subsystem [11:48:21:121] [5109:0a016500] [ERROR][com.winpr.synch.timer] - InitializeWaitableTimer: os specific implementation is missing --------------------------------------------------------------------------
I'm just shooting in the dark here, because of a lack of anything to test with; there are two ways of getting that error message: - you hit a code path that wants timerfd - you hit a code path that wants POSIX timers Using timerfd would require extra ports-dependencies (libepoll-shim, it seems) and some CMake work. Using POSIX timers requires claiming that more OSsen than just Linux have them. Try the attached patch along with what Kyle already has; it might help a little.
Created attachment 192843 [details] Patch timer implementation
(In reply to Adriaan de Groot from comment #12) did not help, unfortunately; it still complains about missing OS specific implementation. So it seems that it really needs "timerfd"...
(In reply to Nils Beyer from comment #14) Hi, I'm drafting up a new update patch that pulls in epoll-shim for the timerfd implementation. Like Adriaan, I have no way to test this part- I'll make sure to confirm that it at least detects timerfd and builds properly before handing it off to you for testing.
Created attachment 193015 [details] svn(1) diff of net/freerdp Attached is a new patch against net/freerdp on a clean ports tree - can you give that a shot, please? This version pulls in libepoll-shim on a version of FreeRDP that's been fixed to detect it in our environment. I've personally tested the timing mechanism, so that roadblock *should* be out of the way. Do note that all but three of the patches in files/ have been deleted here. I've spent the past couple of days upstreaming all of our patches that I can- most of them have been accepted, I've converted the rest to PATCH_FILES mechanism for the time being as I have pull requests opened upstream for them.
(In reply to Kyle Evans from comment #16) thank you very much for the patch. Works flawlessly (apart from sound) here now. Very nice...
(In reply to Nils Beyer from comment #17) Awesome, good to hear! I have some other things I want to sneak in before I can commit to this update, but hopefully I'll finish with those within the week. =) RE: Sound- I personally don't use freerdp+sound, but I can take a look after this other stuff. They just committed a major refactoring of sound channel bits and apparently added an FFMPEG backend for some of this stuff.
I tested it successfully, but looks like patch is outdated now. abishai@poudriere:/usr/local/poudriere/ports/default/net/freerdp % doas patch -sEC < /home/abishai/rdp.patch 1 out of 5 hunks failed while patching Makefile Probably, we should commit it already and thanks for solution!
(This PR pings kde@ because it's filed against net/krdc, but it's basically blocked on the updates to net/freerdp which were developed in the course of the comments stream here)
Proposed changes had been upstreamed and port updated. I can confirm that rdp feature works.
Closing based on comment #21 and r478200