[Spice-devel] [PATCH spice 9/9] spicec: add controller
Hans de Goede
hdegoede at redhat.com
Sun Oct 17 14:38:31 PDT 2010
Hi,
See 1 comment inline.
On 10/17/2010 04:13 PM, Arnon Gilboa wrote:
<snip>
> @@ -2026,7 +2121,7 @@ void Application::register_channels()
>
> bool Application::process_cmd_line(int argc, char** argv)
> {
> - std::string host;
> + std::string host = "";
> int sport = -1;
> int port = -1;
> bool auto_display_res = false;
> @@ -2050,6 +2145,7 @@ bool Application::process_cmd_line(int argc, char** argv)
> SPICE_OPT_CANVAS_TYPE,
> SPICE_OPT_DISPLAY_COLOR_DEPTH,
> SPICE_OPT_DISABLE_DISPLAY_EFFECTS,
> + SPICE_OPT_CONTROLLER,
> };
>
> #ifdef USE_GUI
> @@ -2068,7 +2164,6 @@ bool Application::process_cmd_line(int argc, char** argv)
> CmdLineParser parser("Spice client", false);
>
> parser.add(SPICE_OPT_HOST, "host", "spice server address", "host", true, 'h');
> - parser.set_reqired(SPICE_OPT_HOST);
> parser.add(SPICE_OPT_PORT, "port", "spice server port", "port", true, 'p');
> parser.add(SPICE_OPT_SPORT, "secure-port", "spice server secure port", "port", true, 's');
> parser.add(SPICE_OPT_SECURE_CHANNELS, "secure-channels",
> @@ -2107,6 +2202,8 @@ bool Application::process_cmd_line(int argc, char** argv)
> "disable guest display effects", "wallpaper/font-smooth/animation/all", true);
> parser.set_multi(SPICE_OPT_DISABLE_DISPLAY_EFFECTS, ',');
>
> + parser.add(SPICE_OPT_CONTROLLER, "controller", "enable external controller");
> +
> for (int i = SPICE_CHANNEL_MAIN; i< SPICE_END_CHANNEL; i++) {
> _peer_con_opt[i] = RedPeer::ConnectionOptions::CON_OP_INVALID;
> }
> @@ -2203,6 +2300,15 @@ bool Application::process_cmd_line(int argc, char** argv)
> return false;
> }
> break;
> + case SPICE_OPT_CONTROLLER:
> + if (argc> 2) {
> + Platform::term_printf("%s: controller cannot be combined with other options\n",
> + argv[0]);
> + _exit_code = SPICEC_ERROR_CODE_INVALID_ARG;
> + return false;
> + }
> + _enable_controller = true;
> + return true;
> case CmdLineParser::OPTION_HELP:
> parser.show_help();
> return false;
> @@ -2214,42 +2320,21 @@ bool Application::process_cmd_line(int argc, char** argv)
> }
> }
>
> + if (host.empty()) {
> + Platform::term_printf("%s: missing --host\n", argv[0]);
> + return false;
> + }
> +
This seems weird, first you only allow SPICE_OPT_CONTROLLER by itself, but now
you expect there to be a hostname too ?
<snip>
> @@ -155,7 +157,8 @@ typedef std::map<int, AppMenuItem> AppMenuItemMap;
> class Application : public ProcessLoop,
> public Platform::EventListener,
> public Platform::DisplayModeListener,
> - public ForeignMenuInterface {
> + public ForeignMenuInterface,
> + public ControllerInterface {
> public:
>
> enum State {
and ControllerInterface re-appears. I do believe it would be better to simply
leave it in in the previous patch :)
<snip>
> diff --git a/client/controller_prot.h b/client/controller_prot.h
> new file mode 100644
> index 0000000..6cf4ca0
> --- /dev/null
> +++ b/client/controller_prot.h
> @@ -0,0 +1,115 @@
> +/*
> + Copyright (C) 2009 Red Hat, Inc.
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + This library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, see<http://www.gnu.org/licenses/>.
> +*/
> +
> +#ifndef _H_CONTROLLER_PROT
> +#define _H_CONTROLLER_PROT
> +
> +#define CONTROLLER_MAGIC (*(uint32_t*)"CTRL")
> +#define CONTROLLER_VERSION 1
> +
> +#ifdef __GNUC__
> +#define ATTR_PACKED __attribute__ ((__packed__))
> +#else
> +#define ATTR_PACKED __declspec(align(1))
> +#endif
> +
> +typedef struct ATTR_PACKED ControllerInitHeader {
> + uint32_t magic;
> + uint32_t version;
> + uint32_t size;
> +} ControllerInitHeader;
> +
> +typedef struct ATTR_PACKED ControllerInit {
> + ControllerInitHeader base;
> + uint64_t credentials;
> + uint32_t flags;
> +} ControllerInit;
> +
> +enum {
> + CONTROLLER_FLAG_EXCLUSIVE = 1<< 0,
> +};
> +
> +typedef struct ATTR_PACKED ControllerMsg {
> + uint32_t id;
> + uint32_t size;
> +} ControllerMsg;
> +
> +enum {
> + //extrenal app -> spice client
> + CONTROLLER_HOST = 1,
> + CONTROLLER_PORT,
> + CONTROLLER_SPORT,
> + CONTROLLER_PASSWORD,
> +
> + CONTROLLER_SECURE_CHANNELS,
> + CONTROLLER_DISABLE_CHANNELS,
> +
> + CONTROLLER_TLS_CIPHERS,
> + CONTROLLER_CA_FILE,
> + CONTROLLER_HOST_SUBJECT,
> +
> + CONTROLLER_FULL_SCREEN,
> + CONTROLLER_SET_TITLE,
> +
> + CONTROLLER_CREATE_MENU,
> + CONTROLLER_DELETE_MENU,
> +
> + CONTROLLER_HOTKEYS,
> + CONTROLLER_SEND_CAD,
> +
> + CONTROLLER_CONNECT,
> + CONTROLLER_SHOW,
> + CONTROLLER_HIDE,
> +
> + //spice client -> extrenal app
> + CONTROLLER_MENU_ITEM_CLICK = 1001,
> +};
> +
> +#define CONTROLLER_TRUE (1<< 0)
> +
> +enum {
> + CONTROLLER_SET_FULL_SCREEN = CONTROLLER_TRUE,
> + CONTROLLER_AUTO_DISPLAY_RES = 1<< 1,
> +};
> +
> +typedef struct ATTR_PACKED ControllerValue {
> + ControllerMsg base;
> + uint32_t value;
> +} ControllerValue;
> +
> +typedef struct ATTR_PACKED ControllerData {
> + ControllerMsg base;
> + uint8_t data[0];
> +} ControllerData;
> +
> +#define CONTROLLER_MENU_ITEM_DELIMITER L"\n"
> +#define CONTROLLER_MENU_PARAM_DELIMITER L"\r"
> +
> +enum {
> + CONTROLLER_MENU_FLAGS_SEPARATOR = 1<< 0,
> + CONTROLLER_MENU_FLAGS_DISABLED = 1<< 1,
> + CONTROLLER_MENU_FLAGS_POPUP = 1<< 2,
> + CONTROLLER_MENU_FLAGS_CHECKED = 1<< 3,
> + CONTROLLER_MENU_FLAGS_GRAYED = 1<< 4,
> +};
> +
> +#define SPICE_MENU_INTERNAL_ID_BASE 0x1300
> +#define SPICE_MENU_INTERNAL_ID_SHIFT 8
> +
> +#undef ATTR_PACKED
> +
> +#endif
another candidate for spice-protocol.
Regards,
Hans
More information about the Spice-devel
mailing list