Created attachment 74182 [details] [review]
Added option to user modify the port

My first patch to vino. It adds the ability to user modify the port.

Please, try it.

Any suggestion, correction, and so on, feel free to contact me.

Bye,
Jonh Wendell <wendell@bani.com.br>

Created attachment 74361 [details] [review]
Added option to user modify the port

ChangeLog:

2006-10-06  Jonh Wendell  <wendell@bani.com.br>

* server/vino-server.[ch],
* server/vino-prefs.c,
* server/vino-server.schemas.in,
* capplet/vino-preferences.c,
* capplet/vino-preferences.glade: Added ability to user choose a different port
Closes bug #333752.

The patch looks pretty cool. Good job.

The patch needs just a few changes and it can be committed. Do you have a CVS account? If so, just fix up the few things I suggest below, go ahead and commit to HEAD and close the bug. I've created a gnome-2-16 branch because since this is a new feature, it should only go into 2.17.x.

First, I'm not too sure about adding a UI for this preference. We might just leave it as accessible through GConf only. What I'd suggest is that you don't commit the UI part and open another bug "Additions to preferences dialog". In that bug we can discuss how best to add this preference and the other "advanced" preference that there's a patch for "only allow connections from localhost" (bug #156242). Hopefully Calum can sort it out for us. A screenshot of what you propose would help.

Also, I noticed one of the GConf notify handlers does this:

  if (entry->value->type != GCONF_VALUE_INT)
    return

You really want this:

  if (!entry->value || entry->value->type != GCONF_VALUE_INT)
    return

The reason is that if you unset a GConf key, GConf will invoke the notify handler with entry->value == NULL which would cause a crash with the first version.

There were a couple of places where you had the likes of:

  static void
  vino_server_set_use_alternative_port (VinoServer *server,
                          gboolean use_alternative_port)

or

  static void vino_server_set_frobnicator (VinoServer *server,
                                           int         frobnicator);
  static void vino_server_set_use_alternative_port (VinoServer *server.
                                           gboolean    use_alternative_port);
  
I'd prefer if you fixed them up ala:

  static void
  vino_server_set_use_alternative_port (VinoServer *server,
                                        gboolean    use_alternative_port)

and

  static void vino_server_set_frobnicator          (VinoServer *server,
                                                    int         frobnicator);
  static void vino_server_set_use_alternative_port (VinoServer *server.
                                                    gboolean    use_alterna...);

even if that means re-indenting a whole pile of other prototypes in the
header.

Thanks again John

2006-10-13  Mark McLoughlin  <mark@skynet.ie>

        Add "use_alternative_port" and "alternative_port" GConf
        keys, allowing people to specify a specific port to listen
        on.

        Based on patch from John Wendell <wendell_listas@bani.com.br>
        in bug #333752.

        * vino/server/vino-server.schemas.in: add new keys.

        * vino/server/vino-prefs.c:
        (vino_prefs_use_alternative_port_changed),
        (vino_prefs_alternative_port_changed): handle changes to new keys
        (vino_prefs_create_server): create server with new props
        (vino_prefs_init): read new keys and set up notify handlers

        * vino/server/vino-server.[ch]:
        (vino_server_init_from_screen): explictly set port and turn
        off autoPort if use_alternative_port is set.
        (vino_server_set_property), (vino_server_get_property),
        (vino_server_class_init): add new properties.
        (vino_server_get_use_alternative_port),
        (vino_server_set_use_alternative_port),
        (vino_server_get_alternative_port),
        (vino_server_set_alternative_port): add accessors - re-init
        the listening port if things change after the server is running.

        * vino/server/libvncserver/sockets.c:
        (rfbInitSockets): split out some of this into
        (rfbInitListenSock): this.
        (rfbSetAutoPort), (rfbSetPort): add these two.

        * vino/server/libvncserver/rfb/rfb.h: add rfbSetAutoPort()
        and rfbSetPort().

        * vino/server/libvncserver/CHANGES: add note.

I've logged bug #361891 to discussion how we want to change the preferences dialog

*** Bug 365461 has been marked as a duplicate of this bug. ***