[Spice-devel,spice-gtk,Win32,v3,02/12] NamedPipe: spice_named_pipe_new: add param to distinguish Server or Client

Submitted by Uri Lublin on June 28, 2012, 1:46 a.m.

Details

Message ID 1340848001-7791-3-git-send-email-uril@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Uri Lublin June 28, 2012, 1:46 a.m.
Currently both users are servers.
---
 gtk/controller/namedpipe.c                   |   24 +++++++++++++++++-------
 gtk/controller/namedpipe.h                   |    4 +++-
 gtk/controller/spice-controller-listener.c   |    2 +-
 gtk/controller/spice-foreign-menu-listener.c |    2 +-
 4 files changed, 22 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

diff --git a/gtk/controller/namedpipe.c b/gtk/controller/namedpipe.c
index 355887a..20dde4c 100644
--- a/gtk/controller/namedpipe.c
+++ b/gtk/controller/namedpipe.c
@@ -80,12 +80,21 @@  spice_named_pipe_constructed (GObject *object)
        pipe, in overlapped mode */
     goto end;

-  np->priv->handle = CreateNamedPipe (np->priv->name,
-      PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
-      PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
-      PIPE_UNLIMITED_INSTANCES,
-      DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
-      0, NULL);
+  if (np->priv->is_server) {
+      np->priv->handle = CreateNamedPipe (np->priv->name,
+          PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
+          PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
+          PIPE_UNLIMITED_INSTANCES,
+          DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
+          0, NULL);
+  } else {
+      np->priv->handle = CreateFile (np->priv->name,
+          GENERIC_READ | GENERIC_WRITE,
+          0, NULL,
+          OPEN_EXISTING,
+          FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED,
+          NULL);
+  }

   if (np->priv->handle == INVALID_HANDLE_VALUE)
     {
@@ -244,11 +253,12 @@  spice_named_pipe_initable_iface_init (GInitableIface *iface)
 }

 SpiceNamedPipe *
-spice_named_pipe_new (const gchar *name, GError **error)
+spice_named_pipe_new (const gchar *name, const gboolean is_server, GError **error)
 {
   return SPICE_NAMED_PIPE (g_initable_new (SPICE_TYPE_NAMED_PIPE,
                                            NULL, error,
                                            "name", name,
+                                           "isserver", is_server,
                                            NULL));
 }

diff --git a/gtk/controller/namedpipe.h b/gtk/controller/namedpipe.h
index e0e873b..59f5618 100644
--- a/gtk/controller/namedpipe.h
+++ b/gtk/controller/namedpipe.h
@@ -50,7 +50,9 @@  struct _SpiceNamedPipe

 GType            spice_named_pipe_get_type  (void) G_GNUC_CONST;

-SpiceNamedPipe * spice_named_pipe_new       (const gchar *name, GError **error);
+SpiceNamedPipe * spice_named_pipe_new       (const gchar *name,
+                                             const gboolean is_server,
+                                             GError **error);
 void *           spice_named_pipe_get_handle(SpiceNamedPipe *namedpipe);
 gboolean         spice_named_pipe_close     (SpiceNamedPipe *namedpipe,
                                              GError **error);
diff --git a/gtk/controller/spice-controller-listener.c b/gtk/controller/spice-controller-listener.c
index da1121e..202d516 100644
--- a/gtk/controller/spice-controller-listener.c
+++ b/gtk/controller/spice-controller-listener.c
@@ -89,7 +89,7 @@  spice_controller_listener_new (const gchar *address, GError **error)

         listener = G_OBJECT (spice_named_pipe_listener_new ());

-        np = spice_named_pipe_new (addr, error);
+        np = spice_named_pipe_new (addr, TRUE, error);
         if (!np) {
             g_object_unref (listener);
             listener = NULL;
diff --git a/gtk/controller/spice-foreign-menu-listener.c b/gtk/controller/spice-foreign-menu-listener.c
index 8322a13..53e03e7 100644
--- a/gtk/controller/spice-foreign-menu-listener.c
+++ b/gtk/controller/spice-foreign-menu-listener.c
@@ -91,7 +91,7 @@  spice_foreign_menu_listener_new (const gchar *address, GError **error)

         listener = G_OBJECT (spice_named_pipe_listener_new ());

-        np = spice_named_pipe_new (addr, error);
+        np = spice_named_pipe_new (addr, TRUE, error);
         if (!np) {
             g_object_unref (listener);
             listener = NULL;

Comments

ack

Uri Lublin wrote:
> Currently both users are servers.
> ---
>  gtk/controller/namedpipe.c                   |   24 +++++++++++++++++-------
>  gtk/controller/namedpipe.h                   |    4 +++-
>  gtk/controller/spice-controller-listener.c   |    2 +-
>  gtk/controller/spice-foreign-menu-listener.c |    2 +-
>  4 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/gtk/controller/namedpipe.c b/gtk/controller/namedpipe.c
> index 355887a..20dde4c 100644
> --- a/gtk/controller/namedpipe.c
> +++ b/gtk/controller/namedpipe.c
> @@ -80,12 +80,21 @@ spice_named_pipe_constructed (GObject *object)
>         pipe, in overlapped mode */
>      goto end;
>
> -  np->priv->handle = CreateNamedPipe (np->priv->name,
> -      PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
> -      PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
> -      PIPE_UNLIMITED_INSTANCES,
> -      DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
> -      0, NULL);
> +  if (np->priv->is_server) {
> +      np->priv->handle = CreateNamedPipe (np->priv->name,
> +          PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
> +          PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
> +          PIPE_UNLIMITED_INSTANCES,
> +          DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
> +          0, NULL);
> +  } else {
> +      np->priv->handle = CreateFile (np->priv->name,
> +          GENERIC_READ | GENERIC_WRITE,
> +          0, NULL,
> +          OPEN_EXISTING,
> +          FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED,
> +          NULL);
> +  }
>
>    if (np->priv->handle == INVALID_HANDLE_VALUE)
>      {
> @@ -244,11 +253,12 @@ spice_named_pipe_initable_iface_init (GInitableIface *iface)
>  }
>
>  SpiceNamedPipe *
> -spice_named_pipe_new (const gchar *name, GError **error)
> +spice_named_pipe_new (const gchar *name, const gboolean is_server, GError **error)
>  {
>    return SPICE_NAMED_PIPE (g_initable_new (SPICE_TYPE_NAMED_PIPE,
>                                             NULL, error,
>                                             "name", name,
> +                                           "isserver", is_server,
>                                             NULL));
>  }
>
> diff --git a/gtk/controller/namedpipe.h b/gtk/controller/namedpipe.h
> index e0e873b..59f5618 100644
> --- a/gtk/controller/namedpipe.h
> +++ b/gtk/controller/namedpipe.h
> @@ -50,7 +50,9 @@ struct _SpiceNamedPipe
>
>  GType            spice_named_pipe_get_type  (void) G_GNUC_CONST;
>
> -SpiceNamedPipe * spice_named_pipe_new       (const gchar *name, GError **error);
> +SpiceNamedPipe * spice_named_pipe_new       (const gchar *name,
> +                                             const gboolean is_server,
> +                                             GError **error);
>  void *           spice_named_pipe_get_handle(SpiceNamedPipe *namedpipe);
>  gboolean         spice_named_pipe_close     (SpiceNamedPipe *namedpipe,
>                                               GError **error);
> diff --git a/gtk/controller/spice-controller-listener.c b/gtk/controller/spice-controller-listener.c
> index da1121e..202d516 100644
> --- a/gtk/controller/spice-controller-listener.c
> +++ b/gtk/controller/spice-controller-listener.c
> @@ -89,7 +89,7 @@ spice_controller_listener_new (const gchar *address, GError **error)
>
>          listener = G_OBJECT (spice_named_pipe_listener_new ());
>
> -        np = spice_named_pipe_new (addr, error);
> +        np = spice_named_pipe_new (addr, TRUE, error);
>          if (!np) {
>              g_object_unref (listener);
>              listener = NULL;
> diff --git a/gtk/controller/spice-foreign-menu-listener.c b/gtk/controller/spice-foreign-menu-listener.c
> index 8322a13..53e03e7 100644
> --- a/gtk/controller/spice-foreign-menu-listener.c
> +++ b/gtk/controller/spice-foreign-menu-listener.c
> @@ -91,7 +91,7 @@ spice_foreign_menu_listener_new (const gchar *address, GError **error)
>
>          listener = G_OBJECT (spice_named_pipe_listener_new ());
>
> -        np = spice_named_pipe_new (addr, error);
> +        np = spice_named_pipe_new (addr, TRUE, error);
>          if (!np) {
>              g_object_unref (listener);
>              listener = NULL;
>
Hi

> Uri Lublin wrote:
>>
>> Currently both users are servers.
>> ---
>>  gtk/controller/namedpipe.c                   |   24
>> +++++++++++++++++-------
>>  gtk/controller/namedpipe.h                   |    4 +++-
>>  gtk/controller/spice-controller-listener.c   |    2 +-
>>  gtk/controller/spice-foreign-menu-listener.c |    2 +-
>>  4 files changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/gtk/controller/namedpipe.c b/gtk/controller/namedpipe.c
>> index 355887a..20dde4c 100644
>> --- a/gtk/controller/namedpipe.c
>> +++ b/gtk/controller/namedpipe.c
>> @@ -80,12 +80,21 @@ spice_named_pipe_constructed (GObject *object)
>>        pipe, in overlapped mode */
>>     goto end;
>>
>> -  np->priv->handle = CreateNamedPipe (np->priv->name,
>> -      PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
>> -      PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
>> -      PIPE_UNLIMITED_INSTANCES,
>> -      DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
>> -      0, NULL);
>> +  if (np->priv->is_server) {
>> +      np->priv->handle = CreateNamedPipe (np->priv->name,
>> +          PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
>> +          PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
>> +          PIPE_UNLIMITED_INSTANCES,
>> +          DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
>> +          0, NULL);
>> +  } else {
>> +      np->priv->handle = CreateFile (np->priv->name,
>> +          GENERIC_READ | GENERIC_WRITE,
>> +          0, NULL,
>> +          OPEN_EXISTING,
>> +          FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED,
>> +          NULL);
>> +  }
>>

Are you sure you need namedpipe object for that?

I think you could have used regular GWin32InputStream GWin32OutputStream..

(that was my initial design, iirc)
On 06/28/2012 01:31 PM, Marc-André Lureau wrote:
>>> Currently both users are servers.
>>> ---
>>>   gtk/controller/namedpipe.c                   |   24
>>> +++++++++++++++++-------
>>>   gtk/controller/namedpipe.h                   |    4 +++-
>>>   gtk/controller/spice-controller-listener.c   |    2 +-
>>>   gtk/controller/spice-foreign-menu-listener.c |    2 +-
>>>   4 files changed, 22 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/gtk/controller/namedpipe.c b/gtk/controller/namedpipe.c
>>> index 355887a..20dde4c 100644
>>> --- a/gtk/controller/namedpipe.c
>>> +++ b/gtk/controller/namedpipe.c
>>> @@ -80,12 +80,21 @@ spice_named_pipe_constructed (GObject *object)
>>>         pipe, in overlapped mode */
>>>      goto end;
>>>
>>> -  np->priv->handle = CreateNamedPipe (np->priv->name,
>>> -      PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
>>> -      PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
>>> -      PIPE_UNLIMITED_INSTANCES,
>>> -      DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
>>> -      0, NULL);
>>> +  if (np->priv->is_server) {
>>> +      np->priv->handle = CreateNamedPipe (np->priv->name,
>>> +          PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED,
>>> +          PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
>>> +          PIPE_UNLIMITED_INSTANCES,
>>> +          DEFAULT_PIPE_BUF_SIZE, DEFAULT_PIPE_BUF_SIZE,
>>> +          0, NULL);
>>> +  } else {
>>> +      np->priv->handle = CreateFile (np->priv->name,
>>> +          GENERIC_READ | GENERIC_WRITE,
>>> +          0, NULL,
>>> +          OPEN_EXISTING,
>>> +          FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED,
>>> +          NULL);
>>> +  }
>>>
> Are you sure you need namedpipe object for that?
>
> I think you could have used regular GWin32InputStream GWin32OutputStream..
>
> (that was my initial design, iirc)

I'm not sure I really need a namedpipe object.
It's one way to implement it, using NamedPipe and NamedPipe connection 
and GIOStream.
Does GWin32InputStream/OutputStream better ?  In what way ?

Note that the NamedPipe class was only acting as server.
These first two patches implements a NamedPipe client.

Thanks,
     Uri.
El jue, 28-06-2012 a las 14:27 +0300, Uri Lublin escribió:
> > Are you sure you need namedpipe object for that?
> >
> > I think you could have used regular GWin32InputStream GWin32OutputStream..
> >
> > (that was my initial design, iirc)
> 
> I'm not sure I really need a namedpipe object.
> It's one way to implement it, using NamedPipe and NamedPipe connection 
> and GIOStream.
> Does GWin32InputStream/OutputStream better ?  In what way ?

GWin32InputStream/OutputStream are used by the NamedPipeConnection.

The difference is GWin32IOStream is more generic, as it can take any
handle. This allows you to create it any way you want (though it should
be at least OVERLAPPED to be really async, iirc)

> Note that the NamedPipe class was only acting as server.
> These first two patches implements a NamedPipe client.

Yeah, I think it was made like this by design. In fact, I think it
should be a private object to NamedPipeListener.

I would avoid reusing NamedPipe for clients. It shouldn't be difficult
to change, sorry for not noticing that before.