[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #320: GUACAMOLE-1245: Add support for specifying Wake-on-LAN port.

[GitBox <gi...@apache.org>]

mike-jumper commented on a change in pull request #320:
URL: https://github.com/apache/guacamole-server/pull/320#discussion_r549827673



##########
File path: src/libguac/guacamole/wol-constants.h
##########
@@ -39,13 +39,5 @@
  */
 #define GUAC_WOL_PACKET_SIZE 102
 
-/**
- * The port number that the magic packet should contain as the destination.  In
- * reality this doesn't matter all that much, since the packet is not usually
- * processed by a full IP stack, but defining one is considered a standard
- * practice.
- */
-#define GUAC_WOL_PORT 9
-

Review comment:
       Why remove the port constant from libguac?

##########
File path: src/libguac/guacamole/wol.h
##########
@@ -42,11 +42,15 @@
  * @param broadcast_addr
  *     The broadcast address to which to send the magic Wake-on-LAN packet.
  * 
+ * @param broadcast_port
+ *     The UDP port to use when sending the WoL packet.
+ * 
  * @return 
  *     Zero if the packet is successfully sent to the destination; non-zero
  *     if the packet cannot be sent.
  */
-int guac_wol_wake(const char* mac_addr, const char* broadcast_addr);
+int guac_wol_wake(const char* mac_addr, const char* broadcast_addr,
+        const unsigned short broadcast_port);

Review comment:
       While the address for the WoL packet is expected to be the broadcast address, I don't believe there is such a thing as a "broadcast port".

##########
File path: src/protocols/telnet/settings.c
##########
@@ -63,6 +63,7 @@ const char* GUAC_TELNET_CLIENT_ARGS[] = {
     "wol-send-packet",
     "wol-mac-addr",
     "wol-broadcast-addr",
+    "wol-broadcast-port",

Review comment:
       As with the `broadcast_port` parameter, I suspect that `wol-broadcast-port` may be technically incorrect, and that something like `wol-port` would be preferable.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org