[GitHub] incubator-guacamole-server pull request #120: GUACAMOLE-424: Fix null pointe...

[itsankoff <gi...@git.apache.org>]

Github user itsankoff commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-server/pull/120#discussion_r148159347
  
    --- Diff: src/protocols/vnc/user.c ---
    @@ -112,8 +112,10 @@ int guac_vnc_user_leave_handler(guac_user* user) {
     
         guac_vnc_client* vnc_client = (guac_vnc_client*) user->client->data;
     
    -    /* Update shared cursor state */
    -    guac_common_cursor_remove_user(vnc_client->display->cursor, user);
    +    if (vnc_client && vnc_client->display && vnc_client->display->cursor) {
    --- End diff --
    
    Yeah I totally agree about blindly checking for NULL values. I also agree about the `vnc_client` check and I will remove it. But for the `cursor` there is actually a rare case when cursor allocation can fail and cursor within a valid display to be `NULL` (https://github.com/apache/incubator-guacamole-server/blob/95be88be19e04e07ac1dafb823993745bee7d146/src/common/cursor.c#L40-L41). That's why I think with these thoughts in mind the final check should be:
    ```
    if (vnc_client->display && vnc_client->display->cursor) {
    ...
    }
    ```


---