You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2020/03/18 17:32:33 UTC

[GitHub] [guacamole-server] necouchman opened a new pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

necouchman opened a new pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267
 
 
   Okay, rough start, here, I'm sure, but hopefully not too bad :-).  I used a couple of examples I found online to guide me in writing my code, so hopefully it makes sense.  Let me know what needs to be fixed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r395015330
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
 
 Review comment:
   Okay, taken a stab at IPv4 and IPv6 support concurrently, so see if that looks okay.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r407910688
 
 

 ##########
 File path: src/libguac/guacamole/wol-constants.h
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __GUAC_WOL_CONSTANTS_H
+#define __GUAC_WOL_CONSTANTS_H
+
+/**
+ * Header file that provides constants and defaults related to libguac
+ * Wake-on-LAN support.
+ * 
+ * @file wol-constants.h
+ */
+
+/**
+ * The default broadcast address to which Wake-on-LAN packets will be sent
+ * if one is not specified.
+ */
+#define GUAC_WOL_DEFAULT_BROADCAST "255.255.255.255"
 
 Review comment:
   The reason I have issue with "default" here is that we're essentially _recommending_ that behavior be implemented as a default, not actually implementing a default. It feels like libguac should not concern itself with what protocol implementations may choose to be their defaults.
   
   If some aspect of this WoL implementation implemented its own defaults in the case that specific values are omitted (if `NULL` is passed for the address, a negative value for the timeout, etc.), then I completely agree with documenting these as defaults within libguac since that's exactly what they would be - libguac's defaults. Outside of that, I think the aspect of these values being defaults (or not) would be a private concern of the implementation using libguac.
   
   If the main goal with these defaults is to avoid duplicated code within the bundled protocol support, perhaps some sort of private `defaults.h` within `src/common/` would be better?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on issue #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on issue #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#issuecomment-600788901
 
 
   Okay, identified one issue, already - if the wait time is set for too large amount of time, the connection closes down.  Not sure what I need to do to really make it wait for the amount of time and not terminate the connection?  Also, might be good to send a custom message back to the client that it can display during this time, if that's possible?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394537195
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
+    
+    /* Send the packet and return number of bytes sent. */
+    return sendto(wol_socket, &packet, sizeof(unsigned char) * 102, 0,
+            (struct sockaddr*) &wol_dest, sizeof(wol_dest));
+ 
+}
+
+int guac_wol_wake(const char* mac_addr, const char* broadcast_addr,
+        const int wait_time) {
+    
+    unsigned char wol_packet[102];
 
 Review comment:
   What's this 102 magic number?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394558996
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
+    
+    /* Send the packet and return number of bytes sent. */
+    return sendto(wol_socket, &packet, sizeof(unsigned char) * 102, 0,
+            (struct sockaddr*) &wol_dest, sizeof(wol_dest));
+ 
+}
+
+int guac_wol_wake(const char* mac_addr, const char* broadcast_addr,
+        const int wait_time) {
+    
+    unsigned char wol_packet[102];
 
 Review comment:
   Probably should be a constant, eh?  This is the size of the "magic packet" - https://en.wikipedia.org/wiki/Wake-on-LAN#Magic_packet.  Six bytes of 255, followed by the MAC address repeated 16 times.  Which, come to think of it, I may not have done correctly -  I'll need to double-check that...
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r408853804
 
 

 ##########
 File path: src/libguac/guacamole/wol-constants.h
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __GUAC_WOL_CONSTANTS_H
+#define __GUAC_WOL_CONSTANTS_H
+
+/**
+ * Header file that provides constants and defaults related to libguac
+ * Wake-on-LAN support.
+ * 
+ * @file wol-constants.h
+ */
+
+/**
+ * The default broadcast address to which Wake-on-LAN packets will be sent
+ * if one is not specified.
+ */
+#define GUAC_WOL_DEFAULT_BROADCAST "255.255.255.255"
 
 Review comment:
   Okay, I see your point.  I've renamed them, at this point, such that they are not defaults, anymore.  The boot time one is still a little bit of a gray area - I can move that constant definition out to the individual protocols, if you prefer?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394536224
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
+    
+    /* Send the packet and return number of bytes sent. */
+    return sendto(wol_socket, &packet, sizeof(unsigned char) * 102, 0,
+            (struct sockaddr*) &wol_dest, sizeof(wol_dest));
+ 
+}
+
+int guac_wol_wake(const char* mac_addr, const char* broadcast_addr,
+        const int wait_time) {
+    
+    unsigned char wol_packet[102];
+    unsigned int dest_mac[6];
+    
+    /* Parse mac address and return with error if parsing fails. */
+    if (sscanf(mac_addr, "%x:%x:%x:%x:%x:%x",
+            &(dest_mac[0]), &(dest_mac[1]), &(dest_mac[2]),
+            &(dest_mac[3]), &(dest_mac[4]), &(dest_mac[5])) != 6) {
+        return -1;
+    }
+    
+    /* Generate the magic packet. */
+    __guac_wol_create_magic_packet(wol_packet, dest_mac);
+    
+    /* Send the packet and record bytes sent. */
+    int bytes_sent = __guac_wol_send_packet(broadcast_addr, wol_packet);
+    
+    /* Sleep for the specified amount of time to wait for the remote
+     host to boot. */
+    sleep(wait_time);
 
 Review comment:
   If the wait time is simply feeding a `sleep()`, would it be better for the caller to take up that responsibility and handle their own sleep (if they decide to do so at all)? Is there always a need to sleep prior to making any attempt to connect, or would it be better to allow protocol implementations to decide whether they wish to simply sleep vs. repeatedly connect and retry over a specified time window?
   
   Either way, the function to call here would be libguac's [`guac_timestamp_sleep()`](https://github.com/apache/guacamole-server/blob/fba6ef461c0e1c8d1bd57e782ec9d947d1d04778/src/libguac/guacamole/timestamp.h#L42-L48), which abstracts away platform specifics and sleeps with millisecond granularity.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394559691
 
 

 ##########
 File path: src/libguac/guacamole/wol-constants.h
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __GUAC_WOL_CONSTANTS_H
+#define __GUAC_WOL_CONSTANTS_H
 
 Review comment:
   ok

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394589843
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
+    
+    /* Send the packet and return number of bytes sent. */
+    return sendto(wol_socket, &packet, sizeof(unsigned char) * 102, 0,
+            (struct sockaddr*) &wol_dest, sizeof(wol_dest));
+ 
+}
+
+int guac_wol_wake(const char* mac_addr, const char* broadcast_addr,
+        const int wait_time) {
+    
+    unsigned char wol_packet[102];
 
 Review comment:
   So, I *think* I've done this correctly...although I still need to make it a constant.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r408853935
 
 

 ##########
 File path: src/libguac/Makefile.am
 ##########
 @@ -69,7 +69,9 @@ libguacinc_HEADERS =                  \
     guacamole/user.h                  \
     guacamole/user-constants.h        \
     guacamole/user-fntypes.h          \
-    guacamole/user-types.h
+    guacamole/user-types.h	      \
+    guacamole/wol.h		      \
+    guacamole/wol-constants.h
 
 Review comment:
   Ahem.  Yes.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r407911213
 
 

 ##########
 File path: src/libguac/Makefile.am
 ##########
 @@ -69,7 +69,9 @@ libguacinc_HEADERS =                  \
     guacamole/user.h                  \
     guacamole/user-constants.h        \
     guacamole/user-fntypes.h          \
-    guacamole/user-types.h
+    guacamole/user-types.h	      \
+    guacamole/wol.h		      \
+    guacamole/wol-constants.h
 
 Review comment:
   I recommend aligning the `\` with the lines above.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394592370
 
 

 ##########
 File path: src/libguac/guacamole/wol-constants.h
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __GUAC_WOL_CONSTANTS_H
+#define __GUAC_WOL_CONSTANTS_H
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394559623
 
 

 ##########
 File path: src/libguac/guacamole/wol-constants.h
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __GUAC_WOL_CONSTANTS_H
+#define __GUAC_WOL_CONSTANTS_H
+
+/**
+ * Header file that provides constants and defaults related to libguac
+ * Wake-on-LAN support.
+ * 
+ * @file wol-constants.h
+ */
+
+/**
+ * The default broadcast address to which Wake-on-LAN packets will be sent
+ * if one is not specified.
+ */
+#define GUAC_WOL_DEFAULT_BROADCAST "255.255.255.255"
 
 Review comment:
   The logic that uses this by default is in the protocol-specific parts where the setting is pulled in - if there's nothing specified for the broadcast address parameter, this is used.  I think it should remain the default, though I'm happy to name it whatever.  I will update the documentation to reflect that it is the local broadcast address.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394560471
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
 
 Review comment:
   No, it will not work with IPv6 as it is written.  I should probably put some logic in to detect what is provided and choose the correct `AF_INET`/`AF_INET6` option.
   
   If the address is invalid then the subsequent call to `sendto()` will fail.  But worth some more robust error checking...I'll beef it up.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394593897
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
 
 Review comment:
   Okay, this may take me a little longer to figure out how to do - the `inet_addr()` function appears to be a little outdated and only supports IPv4 (according to the Linux manpage, anyway), and several of the other ones appear to be ones that are not recommended for us, so I've got to figure out a good way to detect IPv4 vs. IPv6 and parse out into the arguments that the `socket` stuff expects.
   
   Also, for what it's worth, IPv6 doesn't support broadcast, only link-local multicast, so I feel like we're going to end up with a couple of different functions for this - one for IPv4 and one for IPv6.  Definitely a little more work to do, here...

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394541510
 
 

 ##########
 File path: src/libguac/guacamole/wol-constants.h
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __GUAC_WOL_CONSTANTS_H
+#define __GUAC_WOL_CONSTANTS_H
+
+/**
+ * Header file that provides constants and defaults related to libguac
+ * Wake-on-LAN support.
+ * 
+ * @file wol-constants.h
+ */
+
+/**
+ * The default broadcast address to which Wake-on-LAN packets will be sent
+ * if one is not specified.
+ */
+#define GUAC_WOL_DEFAULT_BROADCAST "255.255.255.255"
 
 Review comment:
   Unless there is logic within libguac that uses this value automatically, this doesn't feel like a "default" here (though implementations may well use it as such). According to [RFC 919](https://tools.ietf.org/html/rfc919#section-7), 255.255.255.255 is a special broadcast address for strictly the local network. Perhaps `GUAC_WOL_LOCAL_BROADCAST`, `GUAC_WOL_LOCAL_IPV4_BROADCAST`, or something similar would be better?
   
   Alternatively, providing default logic and using this value if `NULL` is given might make sense. If so, and this remains `GUAC_WOL_DEFAULT_BROADCAST`, I recommend documenting the fact that this is "255.255.255.255", the IPv4 broadcast address for the local network.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394557758
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
+    
+    /* Send the packet and return number of bytes sent. */
+    return sendto(wol_socket, &packet, sizeof(unsigned char) * 102, 0,
+            (struct sockaddr*) &wol_dest, sizeof(wol_dest));
+ 
+}
+
+int guac_wol_wake(const char* mac_addr, const char* broadcast_addr,
+        const int wait_time) {
+    
+    unsigned char wol_packet[102];
+    unsigned int dest_mac[6];
+    
+    /* Parse mac address and return with error if parsing fails. */
+    if (sscanf(mac_addr, "%x:%x:%x:%x:%x:%x",
+            &(dest_mac[0]), &(dest_mac[1]), &(dest_mac[2]),
+            &(dest_mac[3]), &(dest_mac[4]), &(dest_mac[5])) != 6) {
+        return -1;
+    }
+    
+    /* Generate the magic packet. */
+    __guac_wol_create_magic_packet(wol_packet, dest_mac);
+    
+    /* Send the packet and record bytes sent. */
+    int bytes_sent = __guac_wol_send_packet(broadcast_addr, wol_packet);
+    
+    /* Sleep for the specified amount of time to wait for the remote
+     host to boot. */
+    sleep(wait_time);
 
 Review comment:
   I'm happy to move the sleep to the caller at the protocol level - I have no argument with that.  i was trying to avoid as much code duplication as possible, but that's fine with me.  I'll move it out to the individual protocols and use the function you mentioned, which will probably also take care of the other issue I posted above with the connection timing out while sleeping.  Thanks!

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on issue #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on issue #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#issuecomment-600795550
 
 
   Thanks for the quick review, @mike-jumper  - I'll try to get some of these items addressed quickly.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394547731
 
 

 ##########
 File path: src/libguac/guacamole/wol-constants.h
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __GUAC_WOL_CONSTANTS_H
+#define __GUAC_WOL_CONSTANTS_H
 
 Review comment:
   Though we (incorrectly) have used a double-underscore prefix for this in the past, doing so is technically not good and intrudes on a reserved namespace.
   
   This should just be `GUAC_WOL_CONSTANTS_H`.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394592291
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
+    
+    /* Send the packet and return number of bytes sent. */
+    return sendto(wol_socket, &packet, sizeof(unsigned char) * 102, 0,
+            (struct sockaddr*) &wol_dest, sizeof(wol_dest));
+ 
+}
+
+int guac_wol_wake(const char* mac_addr, const char* broadcast_addr,
+        const int wait_time) {
+    
+    unsigned char wol_packet[102];
 
 Review comment:
   Okay, size is now a constant.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#discussion_r394548961
 
 

 ##########
 File path: src/libguac/wol.c
 ##########
 @@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "guacamole/wol.h"
+
+/**
+ * Generate the magic Wake-on-LAN (WoL) packet for the specified MAC address
+ * and place it in the character array.
+ * 
+ * @param packet
+ *     The character array that will contain the generated packet.
+ * 
+ * @param mac_address
+ *     The unsigned int representation of the MAC address to place in the packet.
+ */
+static void __guac_wol_create_magic_packet(unsigned char packet[],
+        unsigned int mac_address[]) {
+    
+    int i;
+    unsigned char mac[6];
+    
+    /* Concurrently fill the first part of the packet with 0xFF, and copy the
+     MAC address from the int array to the char array. */
+    for (i = 0; i < 6; i++) {
+        packet[i] = 0xFF;
+        mac[i] = mac_address[i];
+    }
+    
+    /* Copy the MAC address contents into the char array that is storing
+     the rest of the packet. */
+    for (i = 1; i <= 16; i++) {
+        memcpy(&packet[i * 6], &mac, 6 * sizeof(unsigned char));
+    }
+    
+}
+
+/**
+ * Send the magic Wake-on-LAN (WoL) packet to the specified broadcast address,
+ * returning the number of bytes sent, or zero if any error occurred and nothing
+ * was sent.
+ * 
+ * @param broadcast_addr
+ *     The broadcast address to which to send the magic WoL packet.
+ * 
+ * @param packet
+ *     The magic WoL packet to send.
+ * 
+ * @return 
+ *     The number of bytes sent, or zero if nothing could be sent.
+ */
+static ssize_t __guac_wol_send_packet(const char* broadcast_addr,
+        unsigned char packet[]) {
+    
+    int wol_bcast = 1;
+    struct sockaddr_in wol_dest;
+    int wol_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+    
+    /* Attempt to set broadcast; exit with error if this fails. */
+    if (setsockopt(wol_socket, SOL_SOCKET, SO_BROADCAST, &wol_bcast, sizeof(wol_bcast)) < 0) {
+        return 0;
+    }
+    
+    /* Set server endpoint to the broadcast address. */
+    wol_dest.sin_family = AF_INET;
+    wol_dest.sin_port = htons(9);
+    wol_dest.sin_addr.s_addr = inet_addr(broadcast_addr);
 
 Review comment:
   What if the address is invalid? Will this work with IPv6?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on issue #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac

Posted by GitBox <gi...@apache.org>.
necouchman commented on issue #267: GUACAMOLE-513: Implement Wake-on-LAN support in libguac
URL: https://github.com/apache/guacamole-server/pull/267#issuecomment-601185850
 
 
   Okay, I think I've cleaned up all of those initial items.  What else?!

----------------------------------------------------------------
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


With regards,
Apache Git Services