You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by dejan032 <gi...@git.apache.org> on 2018/12/04 07:42:32 UTC
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
GitHub user dejan032 opened a pull request:
https://github.com/apache/guacamole-server/pull/207
GUACAMOLE-673: Create automatically needed directory for file transfer.
Automatically create transfer folder in guacd file system when user is logged in to rdp, so that the transfer network drive can be used immediately without needing to open it first one time to start copying things into and out of it.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dejan032/guacamole-server master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/guacamole-server/pull/207.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #207
----
commit 48974cf79fa629dc63d35665d57cfdc79dd8fda4
Author: Dejan Milovanovic <de...@...>
Date: 2018-12-04T07:31:45Z
GUACAMOLE-673: Create automatically needed directory for file transfer.
----
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Posted by dejan032 <gi...@git.apache.org>.
Github user dejan032 closed the pull request at:
https://github.com/apache/guacamole-server/pull/207
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238669729
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr, char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
+ int create_fs = 0;
+
+ if (rdp_client) {
+ guac_rdp_settings* settings = rdp_client->settings;
+ if (settings) {
+ create_fs = settings->create_drive_path;
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR, "No settings.");
+ }
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR, "No rdp client.");
+ }
+
+ if (create_fs) {
--- End diff --
My guess is that this and a bunch of the code above could be simplified into something like this:
if (rdp_client->settings->create_drive_path)
Since both `rdp_client` and `rdp_client->settings` should be initialized and defined by this point in the code.
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238670994
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr, char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
+ int create_fs = 0;
+
+ if (rdp_client) {
+ guac_rdp_settings* settings = rdp_client->settings;
+ if (settings) {
+ create_fs = settings->create_drive_path;
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR, "No settings.");
+ }
+ }
+ else {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR, "No rdp client.");
+ }
+
+ if (create_fs) {
+
+ /* Get filesystem, return error if no filesystem */
+ guac_rdp_fs* fs = rdp_client->filesystem;
+ if (fs == NULL) {
+ guac_client_log(device->rdpdr->client, GUAC_LOG_ERROR, "No filesystem.");
+ return;
--- End diff --
A couple of issues, here. First, the `rdp_client->filesystem` is already assigned to `device->data` above - not sure there's any reason to re-assign it, here? Second, I don't know that the null check is required, here, either. I'd need to go back and look at the code that calls this `guac_rdpdr_register_fs` function, but I think it might be previously checked?
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238668883
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr, char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
+ int create_fs = 0;
+
+ if (rdp_client) {
--- End diff --
Not sure this check is necessary, here - rdp_client is used previously in here under the assumption that it is not null, so this is probably not necessary.
---
[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/207#discussion_r238667965
--- Diff: src/protocols/rdp/guac_rdpdr/rdpdr_fs_service.c ---
@@ -153,5 +153,42 @@ void guac_rdpdr_register_fs(guac_rdpdrPlugin* rdpdr, char* drive_name) {
/* Init data */
device->data = rdp_client->filesystem;
+ /* Init directory for file transfer */
--- End diff --
Just off the bat, please follow the style used throughout the rest of the code - each tab should be 4 spaces.
See: http://guacamole.apache.org/guac-style/
---