Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.

2015-07-01 Thread Daniel P. Berrange
On Tue, Jun 30, 2015 at 04:44:10PM -0500, Jeremy White wrote:
 This module uses the usbredir protocol and user space tools,
 which are used by the SPICE project.
 
 Signed-off-by: Jeremy White jwh...@codeweavers.com

[snip]

 diff --git a/drivers/usb/usbredir/Kconfig b/drivers/usb/usbredir/Kconfig
 new file mode 100644
 index 000..284fd02
 --- /dev/null
 +++ b/drivers/usb/usbredir/Kconfig
 @@ -0,0 +1,25 @@
 +config USBREDIR
 + tristate USBREDIR support
 + depends on USB  NET
 + ---help---
 +   This enables connecting a remote USB device over IP using
 +  the USBREDIR protocol.  This module provides a sysfs attach
 +  interface which, if given a socket connected to a remote
 +  usbredirserver, will enable the remote device to behave as
 +  though it were connected to the system running this module.
 +
 +  For more information and user space tools, refer to the
 +  USBREDIR project, which can be found at
 +  http://www.spice-space.org/page/UsbRedir.

[snip]

 new file mode 100644
 index 000..217a2e4
 --- /dev/null
 +++ b/drivers/usb/usbredir/README
 @@ -0,0 +1,20 @@
 +USB Redirection Kernel Module
 +
 +This module allows a Linux system to instatiate USB devices
 +that are located on a remote device.  The USB data is transferred
 +over a socket using the USBREDIR protocol, which is generally
 +used in conjunction with the SPICE project.
 +
 +You will need the USBREDIR user space tools.  They can
 +be found at http://www.spice-space.org/page/UsbRedir.
 +
 +To use, start the usbredirserver on a remote system.
 +For example,
 + ./usbredirserver --port 4000 125f:db8a
 +will export my ADATA thumb drive on the remote system.
 +
 +Next, on the local system, connect a socket and relay that to
 +the kernel module.  The connectkernel utility will do this as follows:
 +  ./connectkernel adata4000 my.remote.device.com 4000
 +
 +The device should attach and be usable on the local system.

What is the security story here ? If I am understanding correctly, you have
a userspace helper which opens a socket, and does a connect() to establish
the connection to the remote system, and then tells the kernel to use the
file descriptor associated with the socket.

Assuming that's correct, then this seems to imply that the socket has raw
plain text data being sent/received, and thus precludes the possibility
of running any security protocol like TLS unless the kernel wants to have
an impl of the TLS protocol.

I don't really think it is sensible to be defining  implementing new
network services which can't support strong encryption and authentication.
Rather than passing the file descriptor to the kernel and having it do
the I/O directly, I think it would be better to dissassociate the kernel
from the network transport, and thus leave all sockets layer data I/O
to userspace daemons so they can layer in TLS or SASL or whatever else
is appropriate for the security need.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.

2015-07-01 Thread Jeremy White
 Assuming that's correct, then this seems to imply that the socket has raw
 plain text data being sent/received, and thus precludes the possibility
 of running any security protocol like TLS unless the kernel wants to have
 an impl of the TLS protocol.

Good point.  For completeness, I'll note that, in a Spice use case, the
data would be encrypted by the normal Spice mechanisms.  And it would be
fairly straight forward to write a user space daemon that would accept
TLS and then relay to the unencrypted socket (of course, it would
rewrite everything, which would be inefficient).

 
 I don't really think it is sensible to be defining  implementing new
 network services which can't support strong encryption and authentication.
 Rather than passing the file descriptor to the kernel and having it do
 the I/O directly, I think it would be better to dissassociate the kernel
 from the network transport, and thus leave all sockets layer data I/O
 to userspace daemons so they can layer in TLS or SASL or whatever else
 is appropriate for the security need.

And that would also eliminate the need to copy the parsing code, which
would be a nice improvement.

I considered this approach, but discarded it, perhaps wrongly, when my
google fu suggested that netlink sockets were the best way to connect
user space and a kernel module.  (Because I perceived netlink sockets to
be functionally equivalent to the relay daemon described, above).

From the user space perspective, the usbredir parser has an interface
that exposes about 20 callback functions, which are invoked with
pointers to a variety of structures.  The ideal would be to have a
mechanism to 'call into' kernel space with those varying interfaces.

Would using ioctls be a reasonable way to achieve this?  Is there a
better way?

In the other direction, the usbredir hc provides a range of functions; I
think most interesting are the urb en/dequeue, hub control, and hub
status calls.  Some of that can be handled in the driver; some would
need to be passed on to user space.

My google fu did not lead me to an obvious way to pass this information
to user space.  The approach that comes to mind is to use a signal, or
woken socket, to instruct user space to poll.

I'd appreciate further comments and advice.

Cheers,

Jeremy
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.

2015-07-01 Thread Hans de Goede

Hi,

On 01-07-15 20:31, Jeremy White wrote:

Assuming that's correct, then this seems to imply that the socket has raw
plain text data being sent/received, and thus precludes the possibility
of running any security protocol like TLS unless the kernel wants to have
an impl of the TLS protocol.


Good point.  For completeness, I'll note that, in a Spice use case, the
data would be encrypted by the normal Spice mechanisms.  And it would be
fairly straight forward to write a user space daemon that would accept
TLS and then relay to the unencrypted socket (of course, it would
rewrite everything, which would be inefficient).



I don't really think it is sensible to be defining  implementing new
network services which can't support strong encryption and authentication.
Rather than passing the file descriptor to the kernel and having it do
the I/O directly, I think it would be better to dissassociate the kernel
from the network transport, and thus leave all sockets layer data I/O
to userspace daemons so they can layer in TLS or SASL or whatever else
is appropriate for the security need.


And that would also eliminate the need to copy the parsing code, which
would be a nice improvement.

I considered this approach, but discarded it, perhaps wrongly, when my
google fu suggested that netlink sockets were the best way to connect
user space and a kernel module.  (Because I perceived netlink sockets to
be functionally equivalent to the relay daemon described, above).

 From the user space perspective, the usbredir parser has an interface
that exposes about 20 callback functions, which are invoked with
pointers to a variety of structures.  The ideal would be to have a
mechanism to 'call into' kernel space with those varying interfaces.

Would using ioctls be a reasonable way to achieve this?  Is there a
better way?

In the other direction, the usbredir hc provides a range of functions; I
think most interesting are the urb en/dequeue, hub control, and hub
status calls.  Some of that can be handled in the driver; some would
need to be passed on to user space.

My google fu did not lead me to an obvious way to pass this information
to user space.  The approach that comes to mind is to use a signal, or
woken socket, to instruct user space to poll.

I'd appreciate further comments and advice.


I think it makes sense to have the actual usbredir protocol parsing
in the kernel, and use a netlink interface, this will make it much
easier to deal with protocol extensions (although we have not had
any extensions to the usbredir proto in a while), and will be much
cleaner then an ioctl interface.

I think that Daniel's concern can easily be fixed by rather then
passing the fd of a socket into the kernel to simply forwarding the
data back and forth from a socket opened by userspace into the netlink
socket. This way SSL, SASL or whatever can be put in between, and
you can even built a nice test-suite this way :)

The downside of this is introducing an extra memcpy of all the data,
but an ioctl interface has the same problem and is going to be
unwieldy, so I advice against that.

As for the extra memcpy I would not worry about that, in all the
performance testing I've done it has almost always been all about
latency not throughput.

Regards,

Hans
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.

2015-07-01 Thread Greg KH
On Wed, Jul 01, 2015 at 10:55:49AM -0500, Jeremy White wrote:
 On 07/01/2015 12:44 AM, Greg KH wrote:
  On Tue, Jun 30, 2015 at 10:34:25PM -0500, Jeremy White wrote:
1.  Is the basic premise reasonable?  Is Hans correct in asserting that 
  an
  alternate USB over IP module will be considered?
  
  I have no idea, if it fully replaces the usbip functionality, I don't
  see why that would be rejected.  But why can't you just fix up usbip for
  the issues you find lacking?
 
 This is what Hans said 5 years ago: [1]
 
  
  3) The protocol itself is far from ideal.
  
  Number 3 is the big deal breaker for me. I've looked at the
  (undocumented) protocol by sifting through the source. And it is
  very low level, all it does is shove usb packets back and forth
  over the network. It has no concept of configuration
  setting (the docs say make sure the device is in the right
  configuration before sharing it). No concept of caching things
  like descriptors, active configuration, per interface alt setting,
  etc.
  
  Besides missing a lot of useful smarts the whole one packet at a
  time approach does not really fly when it comes to isoc endpoints.
  As there paper states, the vm-host / guest os drivers need to
  make sure enough packets are submitted / queued at all time
  to gap the network delay / fill the network pipe.
  
  For iso endpoints it makes much more sense to have a start / stop
  stream model, where the usb-host pumpes the urb ringbuffer and
  sends out data received from the usb device to the vm-host
  (isoc input endp case), or sends data received from the vm-host
  (through a buffer to deal with network jitter) to the isoc output
  endpoint.
  
  This also halves the number of packets which need to be
  send over the network, as their is no need for the vm-host to send
  a request for each packet once an input stream has started / for
  the usb-host to send an ack for each delivered packet for an ouput
  stream. It would still send an error when an error occurs, but their
  is no reason to ack all delivered packets. Given the delay
  caused by buffering, etc. not being able to match up the error to
  an exact packet is not important, as from the vm-host emulated usb
  hc (host controller), the packet has long been delivered already.
  
  Instead we will simply report the error to the guest os for the
  next packet enqueued by the guest after receiving notification of
  the error from the usb-host.
 
 The protocol is now documented, so that part is out of date.  I don't
 see any evidence that the bulk of his other concerns have been
 addressed, however.

Because no one has cared to.  Now it seems you care, so I'd prefer to
see someone fix this up instead of adding another protocol that does
much the same thing.

2.  Do I correctly understand that there are no circumstances where 
  copied
  code can be left unmodified?  Even in the case where the copied code is
  working, production code, and the changes would be just for style?
  
  I doubt the changes would just be for style if you are craming it into
  the kernel tree, as it's a totally different environment from any other
  place this code might have been running in before.
 
 Well, the checkpatch.pl reports were all style (and mostly whitespace);
 roughly 3000 of them against 3000 lines of code :-/.  I did review the
 code, looking for areas where I thought it would badly cram into the
 kernel, and I adjusted the few I found (and sent changes upstream).

style matters, as it's a thing with your brain.  You learn patterns and
if the patterns change, you have to do more work and don't see the real
issues involved.  So by ignoring our style you are saying you don't want
anyone else in the kernel community to ever review or work on the code,
which isn't ok.

  Please do the most basic of polite things and fix this up before posting
  things.
 
  It is often difficult for a newcomer to know what the polite thing is, even
  after studying FAQs and documentation.
  
  Did you read Documentation/SubmittingPatches?
 
 Yes, and SubmittingDrivers, SubmitChecklist, every link listed on
 #kernelnewbies, and the entire lkml FAQ as well.
 
 In hindsight, I think it's mostly a failure of common sense on my part.
 
 The one constructive suggestion I would offer is that the 'RFC PATCH' is
 used heavily by the linux kernel community, but I didn't find much
 discussion of it in the documentation or FAQs.  I think I jumped to some
 erroneous conclusions about it's use.  I'm willing to try to add a note
 on that, if that would be helpful.

I personally ignore RFC patches unless they are tiny and in an area that
I happened to be worried about / working on at the moment as it feels
that the submitter doesn't think they are good enough to be merged
as-is, so I'll wait until they feel it is worthy for submission to
review it.

thanks,

greg k-h
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org

Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.

2015-07-01 Thread Hans de Goede

Hi,

On 01-07-15 18:13, Greg KH wrote:

On Wed, Jul 01, 2015 at 10:55:49AM -0500, Jeremy White wrote:

On 07/01/2015 12:44 AM, Greg KH wrote:

On Tue, Jun 30, 2015 at 10:34:25PM -0500, Jeremy White wrote:

   1.  Is the basic premise reasonable?  Is Hans correct in asserting that an
alternate USB over IP module will be considered?


I have no idea, if it fully replaces the usbip functionality, I don't
see why that would be rejected.  But why can't you just fix up usbip for
the issues you find lacking?


This is what Hans said 5 years ago: [1]



3) The protocol itself is far from ideal.

Number 3 is the big deal breaker for me. I've looked at the
(undocumented) protocol by sifting through the source. And it is
very low level, all it does is shove usb packets back and forth
over the network. It has no concept of configuration
setting (the docs say make sure the device is in the right
configuration before sharing it). No concept of caching things
like descriptors, active configuration, per interface alt setting,
etc.

Besides missing a lot of useful smarts the whole one packet at a
time approach does not really fly when it comes to isoc endpoints.
As there paper states, the vm-host / guest os drivers need to
make sure enough packets are submitted / queued at all time
to gap the network delay / fill the network pipe.

For iso endpoints it makes much more sense to have a start / stop
stream model, where the usb-host pumpes the urb ringbuffer and
sends out data received from the usb device to the vm-host
(isoc input endp case), or sends data received from the vm-host
(through a buffer to deal with network jitter) to the isoc output
endpoint.

This also halves the number of packets which need to be
send over the network, as their is no need for the vm-host to send
a request for each packet once an input stream has started / for
the usb-host to send an ack for each delivered packet for an ouput
stream. It would still send an error when an error occurs, but their
is no reason to ack all delivered packets. Given the delay
caused by buffering, etc. not being able to match up the error to
an exact packet is not important, as from the vm-host emulated usb
hc (host controller), the packet has long been delivered already.

Instead we will simply report the error to the guest os for the
next packet enqueued by the guest after receiving notification of
the error from the usb-host.


The protocol is now documented, so that part is out of date.  I don't
see any evidence that the bulk of his other concerns have been
addressed, however.


Because no one has cared to.  Now it seems you care, so I'd prefer to
see someone fix this up instead of adding another protocol that does
much the same thing.


I understand where you are coming from, but usbip is unfixable as it
has no concept of capability negotiation, protocol versioning or some such.

What we need is an usbip v2, and usbredir was written as that, and has been
used in production for years now for redirection of usb devices from
virtual-machine-viewers into qemu based virtual-machines.

I understand that having 2 protocols for one thing is undesirable in
general, but think of this as usb-mass-storage bulk transport vs uas,
or ipv4 vs ipv6 in some cases it just is necessary to do a new
better protocol.

When I designed and implemented usbredir usbip was pretty much dead, but it
got ressurected later. I've never spoken up on this and never attempted to
block usbip's promotion out of staging, as that seemed unfair since no-one
was working on a virtual-hcd driver for the usbredir protocol.

Likewise I think it is unfair if my not speaking up back then now blocks
an usbredir virtual-hcd driver from entering the kernel.

Regards,

Hans
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.

2015-07-01 Thread Greg KH
On Tue, Jun 30, 2015 at 04:44:10PM -0500, Jeremy White wrote:
 This module uses the usbredir protocol and user space tools,
 which are used by the SPICE project.
 
 Signed-off-by: Jeremy White jwh...@codeweavers.com
 ---
  MAINTAINERS |6 +
  drivers/usb/Kconfig |2 +
  drivers/usb/Makefile|1 +
  drivers/usb/usbredir/Kconfig|   25 +
  drivers/usb/usbredir/Makefile   |4 +
  drivers/usb/usbredir/README |   20 +
  drivers/usb/usbredir/TODO   |   12 +
  drivers/usb/usbredir/device.c   |  327 +
  drivers/usb/usbredir/hub.c  |  489 
  drivers/usb/usbredir/main.c |  100 ++
  drivers/usb/usbredir/redir.c|  535 
  drivers/usb/usbredir/rx.c   |   40 +
  drivers/usb/usbredir/strtok_r.c |   69 +
  drivers/usb/usbredir/strtok_r.h |   26 +
  drivers/usb/usbredir/sysfs.c|  145 +++
  drivers/usb/usbredir/tx.c   |  151 +++
  drivers/usb/usbredir/urb.c  |  266 
  drivers/usb/usbredir/usbredir.h |  225 
  drivers/usb/usbredir/usbredirfilter.c   |  294 +
  drivers/usb/usbredir/usbredirfilter.h   |  144 +++
  drivers/usb/usbredir/usbredirparser.c   | 1795 
 +++
  drivers/usb/usbredir/usbredirparser.h   |  386 ++
  drivers/usb/usbredir/usbredirproto-compat.h |   88 ++
  drivers/usb/usbredir/usbredirproto.h|  309 +
  24 files changed, 5459 insertions(+)
  create mode 100644 drivers/usb/usbredir/Kconfig
  create mode 100644 drivers/usb/usbredir/Makefile
  create mode 100644 drivers/usb/usbredir/README
  create mode 100644 drivers/usb/usbredir/TODO
  create mode 100644 drivers/usb/usbredir/device.c
  create mode 100644 drivers/usb/usbredir/hub.c
  create mode 100644 drivers/usb/usbredir/main.c
  create mode 100644 drivers/usb/usbredir/redir.c
  create mode 100644 drivers/usb/usbredir/rx.c
  create mode 100644 drivers/usb/usbredir/strtok_r.c
  create mode 100644 drivers/usb/usbredir/strtok_r.h
  create mode 100644 drivers/usb/usbredir/sysfs.c
  create mode 100644 drivers/usb/usbredir/tx.c
  create mode 100644 drivers/usb/usbredir/urb.c
  create mode 100644 drivers/usb/usbredir/usbredir.h
  create mode 100644 drivers/usb/usbredir/usbredirfilter.c
  create mode 100644 drivers/usb/usbredir/usbredirfilter.h
  create mode 100644 drivers/usb/usbredir/usbredirparser.c
  create mode 100644 drivers/usb/usbredir/usbredirparser.h
  create mode 100644 drivers/usb/usbredir/usbredirproto-compat.h
  create mode 100644 drivers/usb/usbredir/usbredirproto.h

It's pointless to post a patch that you know has problems with it (i.e.
it's not even in proper kernel coding style), as it will never be
reviewed or even looked at.

Please do the most basic of polite things and fix this up before posting
things.

And really, all in one patch?  That too is pretty hard to review...

thanks,

greg k-h
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.

2015-07-01 Thread Greg KH
On Tue, Jun 30, 2015 at 10:34:25PM -0500, Jeremy White wrote:
 
 It's pointless to post a patch that you know has problems with it (i.e.
 it's not even in proper kernel coding style), as it will never be
 reviewed or even looked at.
 
 Thanks for the reply, and I'm sorry for the clumsy ask.
 
 I would still appreciate feedback on two points:
 
   1.  Is the basic premise reasonable?  Is Hans correct in asserting that an
 alternate USB over IP module will be considered?

I have no idea, if it fully replaces the usbip functionality, I don't
see why that would be rejected.  But why can't you just fix up usbip for
the issues you find lacking?

   2.  Do I correctly understand that there are no circumstances where copied
 code can be left unmodified?  Even in the case where the copied code is
 working, production code, and the changes would be just for style?

I doubt the changes would just be for style if you are craming it into
the kernel tree, as it's a totally different environment from any other
place this code might have been running in before.

 Please do the most basic of polite things and fix this up before posting
 things.
 
 It is often difficult for a newcomer to know what the polite thing is, even
 after studying FAQs and documentation.

Did you read Documentation/SubmittingPatches?

 I appreciate your patience (and clue bats) as I try to learn.
 
 
 And really, all in one patch?  That too is pretty hard to review...
 
 Yeah.  I see the point of pain.  I did not see a solution as I formed the
 patch, but I'll try harder before resending.

Remember you need to make this trivial to review in order to get it
accepted.  You have to do extra work because of this because our limited
resource is reviewers and maintainers, not developers.

thanks,

greg k-h
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-common PATCH 1/2] ppc: Add macros for color byte ordering

2015-07-01 Thread Lukas Venhoda
When using image compression on PowerPC architecture, colors are in
wrong order ARGB - BGRA.

This commit introduces macros, that will change the color order
according to machine endianness.
---
 common/pixman_utils.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/common/pixman_utils.h b/common/pixman_utils.h
index dd565ef..f892777 100644
--- a/common/pixman_utils.h
+++ b/common/pixman_utils.h
@@ -27,6 +27,16 @@

 #include draw.h

+#ifdef WORDS_BIGENDIAN
+# define PIXMAN_LE_x8r8g8b8   PIXMAN_b8g8r8x8
+# define PIXMAN_LE_a8r8g8b8   PIXMAN_b8g8r8a8
+# define PIXMAN_LE_r8g8b8 PIXMAN_b8g8r8
+#else
+# define PIXMAN_LE_x8r8g8b8   PIXMAN_x8r8g8b8
+# define PIXMAN_LE_a8r8g8b8   PIXMAN_a8r8g8b8
+# define PIXMAN_LE_r8g8b8 PIXMAN_r8g8b8
+#endif
+
 SPICE_BEGIN_DECLS

 /* This lists all possible 2 argument binary raster ops.
--
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-common PATCH 2/2] ppc: Fix colors on ppc when using QUIC

2015-07-01 Thread Lukas Venhoda
Fixes color order on PowerPC when using QUIC image compression.
---
 common/canvas_base.c  |  6 +++---
 common/canvas_utils.c | 12 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/common/canvas_base.c b/common/canvas_base.c
index 6f48340..31f3cef 100644
--- a/common/canvas_base.c
+++ b/common/canvas_base.c
@@ -406,19 +406,19 @@ static pixman_image_t *canvas_get_quic(CanvasBase 
*canvas, SpiceImage *image,
 switch (type) {
 case QUIC_IMAGE_TYPE_RGBA:
 as_type = QUIC_IMAGE_TYPE_RGBA;
-pixman_format = PIXMAN_a8r8g8b8;
+pixman_format = PIXMAN_LE_a8r8g8b8;
 break;
 case QUIC_IMAGE_TYPE_RGB32:
 case QUIC_IMAGE_TYPE_RGB24:
 as_type = QUIC_IMAGE_TYPE_RGB32;
-pixman_format = PIXMAN_x8r8g8b8;
+pixman_format = PIXMAN_LE_x8r8g8b8;
 break;
 case QUIC_IMAGE_TYPE_RGB16:
 if (!want_original 
 (canvas-format == SPICE_SURFACE_FMT_32_xRGB ||
  canvas-format == SPICE_SURFACE_FMT_32_ARGB)) {
 as_type = QUIC_IMAGE_TYPE_RGB32;
-pixman_format = PIXMAN_x8r8g8b8;
+pixman_format = PIXMAN_LE_x8r8g8b8;
 } else {
 as_type = QUIC_IMAGE_TYPE_RGB16;
 pixman_format = PIXMAN_x1r5g5b5;
diff --git a/common/canvas_utils.c b/common/canvas_utils.c
index 0d1591a..e59ced7 100644
--- a/common/canvas_utils.c
+++ b/common/canvas_utils.c
@@ -160,12 +160,12 @@ pixman_image_t * surface_create(pixman_format_code_t 
format, int width, int heig

 bitmap_info.inf.bmiHeader.biPlanes = 1;
 switch (format) {
-case PIXMAN_a8r8g8b8:
-case PIXMAN_x8r8g8b8:
+case PIXMAN_LE_a8r8g8b8:
+case PIXMAN_LE_x8r8g8b8:
 bitmap_info.inf.bmiHeader.biBitCount = 32;
 nstride = width * 4;
 break;
-case PIXMAN_r8g8b8:
+case PIXMAN_LE_r8g8b8:
 bitmap_info.inf.bmiHeader.biBitCount = 24;
 nstride = SPICE_ALIGN(width * 3, 4);
 break;
@@ -233,11 +233,11 @@ pixman_image_t * surface_create(pixman_format_code_t 
format, int width, int heig
 // NOTE: we assume here that the lz decoders always decode to RGB32.
 int stride = 0;
 switch (format) {
-case PIXMAN_a8r8g8b8:
-case PIXMAN_x8r8g8b8:
+case PIXMAN_LE_a8r8g8b8:
+case PIXMAN_LE_x8r8g8b8:
 stride = width * 4;
 break;
-case PIXMAN_r8g8b8:
+case PIXMAN_LE_r8g8b8:
 // NOTE: LZ4 also decodes to RGB24
 stride = SPICE_ALIGN(width * 3, 4);
 break;
--
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 08/33] codegen: Remove old ptr32 attribute

2015-07-01 Thread Frediano Ziglio
This attribute is not use in code.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/ptypes.py | 4 
 1 file changed, 4 deletions(-)

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 3a307ed..f94fd24 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -105,8 +105,6 @@ valid_attributes={
 # for a switch this indicate that on network
 # will occupy always same size (maximum size required for all members)
 'fixedsize',
-# use 32 bit pointer
-'ptr32',
 }
 
 attributes_with_arguments={
@@ -613,8 +611,6 @@ class Member(Containee):
 self.container = container
 self.member_type = self.member_type.resolve()
 self.member_type.register()
-if self.has_attr(ptr32) and self.member_type.is_pointer():
-self.member_type.set_ptr_size(4)
 for i in propagated_attributes:
 if self.has_attr(i):
 self.member_type.attributes[i] = self.attributes[i]
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 01/33] codegen: Import six module before first use

2015-07-01 Thread Frediano Ziglio
The module was used in the initial try/except so make sure it is
already imported.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/spice_parser.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index d60bb10..80b559b 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -1,3 +1,5 @@
+import six
+
 try:
 from pyparsing import Literal, CaselessLiteral, Word, OneOrMore, 
ZeroOrMore, \
 Forward, delimitedList, Group, Optional, Combine, alphas, nums, 
restOfLine, cStyleComment, \
@@ -8,7 +10,6 @@ except ImportError:
 
 
 from . import ptypes
-import six
 import sys
 
 cvtInt = lambda toks: int(toks[0])
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 07/33] codegen: Do some check on attributes

2015-07-01 Thread Frediano Ziglio
Verify that the attribute is known. This could help for instance to
avoid some future typo mistake.
Also we have a list of attributes we can comment.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/ptypes.py | 69 
 1 file changed, 69 insertions(+)

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 845fa73..3a307ed 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -62,11 +62,76 @@ class FixedSize:
 # other members
 propagated_attributes=[ptr_array, nonnull, chunk]
 
+valid_attributes={
+# write to an array at end of structure
+'end',
+# the C structure contain a pointer to data
+# for instance we want to write an array to an allocated array
+'to_ptr',
+# write output to this C structure
+'ctype',
+# prefix for flags/values enumerations
+'prefix',
+# use in demarshaller to use directly data from message without copy
+'nocopy',
+# store member array in a pointer
+# similar to to_ptr but has an additional argument as C field to
+# store length
+'as_ptr',
+# do not generate marshal code
+# used for last members to be able to marshall them manually
+'nomarshal',
+# ??? not used by python code
+'zero_terminated',
+'marshall',
+# this pointer member cannot be null
+'nonnull',
+# this flags member contains only a single flag
+'unique_flag',
+'ptr_array',
+'outvar',
+# C structure has anonymous member (used in switch)
+'anon',
+'chunk',
+# this channel if conditional to an #ifdef
+'ifdef',
+# write this member as zero on network
+'zero',
+# specify minor version required for these members
+'minor',
+'bytes_count',
+# this attribute does not exists on the network, fill just structure with 
the value
+'virtual',
+# for a switch this indicate that on network
+# will occupy always same size (maximum size required for all members)
+'fixedsize',
+# use 32 bit pointer
+'ptr32',
+}
+
+attributes_with_arguments={
+'ctype',
+'prefix',
+'as_ptr',
+'outvar',
+'ifdef',
+'minor',
+'bytes_count',
+'virtual',
+}
+
 def fix_attributes(attribute_list):
 attrs = {}
 for attr in attribute_list:
 name = attr[0][1:]
 lst = attr[1:]
+if not name in valid_attributes:
+raise Exception(Attribute %s not recognized % name)
+if not name in attributes_with_arguments:
+if len(lst)  0:
+raise Exception(Attribute %s specified with options % name)
+elif len(lst)  1:
+raise Exception(Attribute %s has more than 1 argument % name)
 attrs[name] = lst
 return attrs
 
@@ -139,6 +204,8 @@ class Type:
 _types_by_name[self.name] = self
 
 def has_attr(self, name):
+if not name in valid_attributes:
+raise Exception('attribute %s not expected' % name)
 return name in self.attributes
 
 class TypeRef(Type):
@@ -522,6 +589,8 @@ class Containee:
 return not self.is_switch() and self.member_type.is_primitive()
 
 def has_attr(self, name):
+if not name in valid_attributes:
+raise Exception('attribute %s not expected' % name)
 return name in self.attributes
 
 def has_minor_attr(self):
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 13/33] Decorate writer class to make easier ifdef/endif handling

2015-07-01 Thread Frediano Ziglio
I'm generating code for dissector from demarshaller.
Make simple to hangle ifdef/endif not having to check manually attribute.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 6751534..67d0fa7 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -1,6 +1,8 @@
 
 from . import codegen
 
+import types
+
 
 # generate a new tree identifier
 ett_writer = None
@@ -84,10 +86,26 @@ def write_protocol_definitions(writer):
 writer.end_block()
 
 
+def decorate_writer(writer):
+cls = writer.__class__
+
+def create(old):
+def ifdef(self, member):
+if member.has_attr(ifdef):
+old(self, member.attributes[ifdef][0])
+return types.MethodType(ifdef, None, cls)
+
+cls.ifdef  = create(cls.ifdef)
+cls.ifdef_else = create(cls.ifdef_else)
+cls.endif  = create(cls.endif)
+
+
 def write_protocol_parser(writer, proto):
 global hf_writer
 global ett_writer
 
+decorate_writer(writer)
+
 write_parser_helpers(writer)
 
 # put fields declaration first
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 16/33] Allows to specify descriptions for enumerations

2015-07-01 Thread Frediano Ziglio
These descriptions will be used to show in wireshark dissector.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/ptypes.py   | 16 
 python_modules/spice_parser.py |  2 +-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 0ba5384..ff044fb 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -370,20 +370,24 @@ class EnumType(EnumBaseType):
 last = -1
 names = {}
 values = {}
+descs = {}
 for v in enums:
 name = v[0]
-if len(v)  1:
-value = v[1]
+desc = v[1][1]
+if len(v)  2:
+value = v[2]
 else:
 value = last + 1
 last = value
 
 assert value not in names
 names[value] = name
+descs[value] = desc
 values[name] = value
 
 self.names = names
 self.values = values
+self.descs = descs
 
 self.attributes = fix_attributes(attribute_list)
 
@@ -423,20 +427,24 @@ class FlagsType(EnumBaseType):
 last = -1
 names = {}
 values = {}
+descs = {}
 for v in flags:
 name = v[0]
-if len(v)  1:
-value = v[1]
+desc = v[1][1]
+if len(v)  2:
+value = v[2]
 else:
 value = last + 1
 last = value
 
 assert value not in names
 names[value] = name
+descs[value] = desc
 values[name] = value
 
 self.names = names
 self.values = values
+self.descs = descs
 
 self.attributes = fix_attributes(attribute_list)
 
diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index a0f969a..06000a4 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -120,7 +120,7 @@ def SPICE_BNF():
  int32_ ^ uint32_ ^ int64_ ^ uint64_ ^
  typename).setName(type)
 
-flagsBody = enumBody = Group(lbrace + delimitedList(Group (enumname + 
Optional(equals + integer))) + Optional(comma) + rbrace)
+flagsBody = enumBody = Group(lbrace + delimitedList(Group (enumname + 
Optional(Group(ws_desc), default=[None,None]) + Optional(equals + integer))) + 
Optional(comma) + rbrace)
 
 messageSpec = Group(message_ + messageBody + 
attributes).setParseAction(lambda toks: ptypes.MessageType(None, toks[0][1], 
toks[0][2])) | typename
 
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 09/33] Start adding code to generate wireshark dissector

2015-07-01 Thread Frediano Ziglio
Added a stub dissector.py code.
Added file to Makefiles.
Add option to call dissector and code to call it.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 common/Makefile.am  |  1 +
 python_modules/Makefile.am  |  1 +
 python_modules/dissector.py | 14 ++
 spice_codegen.py| 17 ++---
 4 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 python_modules/dissector.py

diff --git a/common/Makefile.am b/common/Makefile.am
index b4384e8..5e1bffe 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -106,6 +106,7 @@ MARSHALLERS_DEPS =  \
$(top_srcdir)/python_modules/marshal.py \
$(top_srcdir)/python_modules/ptypes.py  \
$(top_srcdir)/python_modules/spice_parser.py\
+   $(top_srcdir)/python_modules/dissector.py   \
$(top_srcdir)/spice_codegen.py  \
$(NULL)
 
diff --git a/python_modules/Makefile.am b/python_modules/Makefile.am
index 50e1a71..5983d5b 100644
--- a/python_modules/Makefile.am
+++ b/python_modules/Makefile.am
@@ -7,6 +7,7 @@ PYTHON_MODULES =\
marshal.py  \
ptypes.py   \
spice_parser.py \
+   dissector.py\
$(NULL)
 
 EXTRA_DIST = $(PYTHON_MODULES)
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
new file mode 100644
index 000..90ba498
--- /dev/null
+++ b/python_modules/dissector.py
@@ -0,0 +1,14 @@
+
+from . import codegen
+
+def write_protocol_parser(writer, proto):
+pass
+
+def write_includes(writer):
+writer.newline()
+writer.writeln('#include epan/packet.h')
+writer.writeln('#include epan/conversation.h')
+writer.writeln('#include epan/expert.h')
+writer.newline()
+writer.writeln('#include packet-spice.h')
+writer.newline()
diff --git a/spice_codegen.py b/spice_codegen.py
index 84790af..8cfec1a 100755
--- a/spice_codegen.py
+++ b/spice_codegen.py
@@ -9,6 +9,7 @@ from python_modules import ptypes
 from python_modules import codegen
 from python_modules import demarshal
 from python_modules import marshal
+from python_modules import dissector
 import six
 
 def write_channel_enums(writer, channel, client, describe):
@@ -110,7 +111,7 @@ parser.add_option(-e, --generate-enums,
   action=store_true, dest=generate_enums, default=False,
   help=Generate enums)
 parser.add_option(-w, --generate-wireshark-dissector,
-  action=store_true, dest=generate_dissector, 
default=False,
+  action=store_true, dest=generate_enum_dissector, 
default=False,
   help=Generate Wireshark dissector definitions)
 parser.add_option(-d, --generate-demarshallers,
   action=store_true, dest=generate_demarshallers, 
default=False,
@@ -118,6 +119,9 @@ parser.add_option(-d, --generate-demarshallers,
 parser.add_option(-m, --generate-marshallers,
   action=store_true, dest=generate_marshallers, 
default=False,
   help=Generate message marshallers)
+parser.add_option(--generate-dissector,
+  action=store_true, dest=generate_dissector, 
default=False,
+  help=Generate dissector)
 parser.add_option(-P, --private-marshallers,
   action=store_true, dest=private_marshallers, 
default=False,
   help=Generate private message marshallers)
@@ -213,8 +217,15 @@ if options.includes:
 writer.header.writeln('#include %s' % i)
 writer.writeln('#include %s' % i)
 
-if options.generate_enums or options.generate_dissector:
-write_enums(writer, options.generate_dissector)
+if options.generate_enums or options.generate_enum_dissector:
+write_enums(writer, options.generate_enum_dissector)
+
+if options.generate_dissector:
+if options.generate_demarshallers:
+print  sys.stderr, You can't specify --generate-demarshallers with 
--generate-dissector
+sys.exit(1)
+dissector.write_includes(writer)
+dissector.write_protocol_parser(writer, proto)
 
 if options.generate_demarshallers:
 if not options.server and not options.client:
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 10/33] Allows to specify C type for index variable

2015-07-01 Thread Frediano Ziglio
This to prepare to generate wireshark dissector which use
glib types instead of new C ones (for compatibility with some
compiler).

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/codegen.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/python_modules/codegen.py b/python_modules/codegen.py
index 02ffdb9..a92bd49 100644
--- a/python_modules/codegen.py
+++ b/python_modules/codegen.py
@@ -81,6 +81,7 @@ class CodeWriter:
 self.has_error_check = False
 self.options = {}
 self.function_helper_writer = None
+self.index_type = 'uint32_t'
 
 def set_option(self, opt, value = True):
 self.options[opt] = value
@@ -113,6 +114,7 @@ class CodeWriter:
 self.contents.append(self.out)
 writer.indentation = self.indentation
 writer.at_line_start = self.at_line_start
+writer.index_type = self.index_type
 writer.generated = self.generated
 writer.options = self.options
 writer.public_prefix = self.public_prefix
@@ -353,7 +355,7 @@ class CodeWriter:
 def pop_index(self):
 index = self.indexes[self.current_index]
 self.current_index = self.current_index + 1
-self.add_function_variable(uint32_t, index)
+self.add_function_variable(self.index_type, index)
 return index
 
 def push_index(self):
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 15/33] Allows to specify some new attributes for wireshark

2015-07-01 Thread Frediano Ziglio
To make output more useful fields from the protocol should have
additional information like description, name, type and so on.

List of attributes added:
- ws_desc, just a simple description
- ws_name name of the field. See below
- ws allow to specify a description and a name. Useful as easy to
  type. If you have a name there is no sense to have no description
- ws_type allows to override type detected, for instance to
  specify is a boolean instead of an int
- ws_base like ws_type but for base (hexadecimal instead of decimal)
- ws_txt and ws_txt_n allows to specify formatting. The difference
  between formatting and description is that description is static
  while these new attributes contain a formatting string as first
  argument followed by arguments representing fields or INDEX for
  the index if we are dumping a structure contained in an array.
  The distinction between ws_txt and ws_txt_n is the item contained
  in an array or not
- ws_inline, this is tricky attribute. It allow to embed structure
  dissecting in the same function. This allow format string and other
  field (for instance switch or array sizes) to be seen by current
  generated function
- ws_as, dissect this structure as another type. Useful to avoid
  changing the protocol but show a slightly modified version.
  This is used for instance to show two fields like x and y as a
  single point. Could also be used to dump a binary data with
  more detail but avoid to change marshalling/demarshalling

These attributes required some changes in the parser as previously
arguments could be only integers and identifiers while they require
string and multiple identifiers (like this.that).

In wireshark names are important as they can be used to do
queries about packet with specific features.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/ptypes.py   | 29 +++--
 python_modules/spice_parser.py | 13 +++--
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index f94fd24..0ba5384 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -105,6 +105,26 @@ valid_attributes={
 # for a switch this indicate that on network
 # will occupy always same size (maximum size required for all members)
 'fixedsize',
+
+# description
+'ws_desc',
+# field name
+'ws_name',
+# combine description + name
+# name will be used for filtering
+'ws',
+# type (BOOLEAN, STRINGZ)
+'ws_type',
+# force wireshark base
+'ws_base',
+# text to use for format, you can specify parameters
+'ws_txt',
+'ws_txt_n',
+# put structure not in a separate function
+# used to be able to retrieve fields from parent
+'ws_inline',
+# handle this container as another type
+'ws_as',
 }
 
 attributes_with_arguments={
@@ -116,6 +136,13 @@ attributes_with_arguments={
 'minor',
 'bytes_count',
 'virtual',
+'ws',
+'ws_desc',
+'ws_type',
+'ws_base',
+'ws_txt',
+'ws_txt_n',
+'ws_as',
 }
 
 def fix_attributes(attribute_list):
@@ -128,8 +155,6 @@ def fix_attributes(attribute_list):
 if not name in attributes_with_arguments:
 if len(lst)  0:
 raise Exception(Attribute %s specified with options % name)
-elif len(lst)  1:
-raise Exception(Attribute %s has more than 1 argument % name)
 attrs[name] = lst
 return attrs
 
diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index 97af8b2..a0f969a 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -3,7 +3,7 @@ import six
 try:
 from pyparsing import Literal, CaselessLiteral, Word, OneOrMore, 
ZeroOrMore, \
 Forward, delimitedList, Group, Optional, Combine, alphas, nums, 
restOfLine, cStyleComment, \
-alphanums, ParseException, ParseResults, Keyword, StringEnd, 
replaceWith
+alphanums, ParseException, ParseResults, Keyword, StringEnd, 
replaceWith, QuotedString
 except ImportError:
 six.print_(Module pyparsing not found.)
 exit(1)
@@ -77,20 +77,29 @@ def SPICE_BNF():
 switch_= Keyword(switch)
 default_   = Keyword(default)
 case_  = Keyword(case)
+ws_= Keyword(@ws)
+ws_txt_= Keyword(@ws_txt) | Keyword(@ws_txt_n)
+ws_desc_   = Keyword(@ws_desc)
 
 identifier = Word( alphas, alphanums + _ )
+multi_identifier = Word( alphas, alphanums + _. )
 enumname = Word( alphanums + _ )
 
 integer = ( Combine( CaselessLiteral(0x) + Word( nums+abcdefABCDEF 
) ) |
 Word( nums++-, nums ) 
).setName(int).setParseAction(cvtInt)
 
+string = QuotedString(quoteChar='', escChar='\\')
+
 typename = identifier.copy().setParseAction(lambda toks : 
ptypes.TypeRef(str(toks[0])))
 
 # This is just normal types, i.e. not 

[Spice-devel] [PATCH 02/33] codegen: Simplify if/else blocks

2015-07-01 Thread Frediano Ziglio
Blocks was mainly the same, reduce code.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/marshal.py | 29 +++--
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/python_modules/marshal.py b/python_modules/marshal.py
index b77b910..cbda2ab 100644
--- a/python_modules/marshal.py
+++ b/python_modules/marshal.py
@@ -380,25 +380,18 @@ def write_protocol_marshaller(writer, proto, is_server, 
private_marshallers):
 writer.ifdef(channel.attributes[ifdef][0])
 writer.header.ifdef(channel.attributes[ifdef][0])
 if is_server:
-for m in channel.client_messages:
-message = m.message_type
-f = write_message_marshaller(writer, message, is_server, 
private_marshallers)
-if channel.has_attr(ifdef) and f not in functions:
-functions[f] = channel.attributes[ifdef][0]
-elif message.has_attr(ifdef) and f not in functions:
-functions[f] = message.attributes[ifdef][0]
-else:
-functions[f] = True
+messages = channel.client_messages
 else:
-for m in channel.server_messages:
-message = m.message_type
-f = write_message_marshaller(writer, message, is_server, 
private_marshallers)
-if channel.has_attr(ifdef) and f not in functions:
-functions[f] = channel.attributes[ifdef][0]
-elif message.has_attr(ifdef) and f not in functions:
-functions[f] = message.attributes[ifdef][0]
-else:
-functions[f] = True
+messages = channel.server_messages
+   for m in messages:
+   message = m.message_type
+   f = write_message_marshaller(writer, message, is_server, 
private_marshallers)
+   if channel.has_attr(ifdef) and f not in functions:
+   functions[f] = channel.attributes[ifdef][0]
+   elif message.has_attr(ifdef) and f not in functions:
+   functions[f] = message.attributes[ifdef][0]
+   else:
+   functions[f] = True
 if channel.has_attr(ifdef):
 writer.endif(channel.attributes[ifdef][0])
 writer.header.endif(channel.attributes[ifdef][0])
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 05/33] codegen: Remove duplicate variable initialization

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/spice_parser.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index 80b559b..97af8b2 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -58,7 +58,6 @@ def SPICE_BNF():
 uint64_= 
Keyword(uint64).setParseAction(replaceWith(ptypes.uint64))
 
 # keywords
-channel_   = Keyword(channel)
 enum32_= Keyword(enum32).setParseAction(replaceWith(32))
 enum16_= Keyword(enum16).setParseAction(replaceWith(16))
 enum8_ = Keyword(enum8).setParseAction(replaceWith(8))
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 03/33] codegen: Fix typo in variable name

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/codegen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python_modules/codegen.py b/python_modules/codegen.py
index f324498..55500b7 100644
--- a/python_modules/codegen.py
+++ b/python_modules/codegen.py
@@ -193,7 +193,7 @@ class CodeWriter:
 def unindent(self):
 self.indentation -= 4
 if self.indentation  0:
-self.indenttation = 0
+self.indentation = 0
 
 def begin_block(self, prefix= , comment = ):
 if len(prefix)  0:
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 18/33] Change code generated index type

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 9971140..598cc3b 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -225,6 +225,8 @@ def write_protocol_parser(writer, proto):
 
 decorate_writer(writer)
 
+writer.index_type = 'guint32'
+
 write_parser_helpers(writer)
 
 # put fields declaration first
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 12/33] Add new_ett function to be able to create new trees

2015-07-01 Thread Frediano Ziglio
Use an array to declare tree items instead of allocating it statically.
This save a bit of memory and it also generate smaller code to read.
A tree in wireshark represent an item which could be expanded.
Possibly wireshark is using these registrations to save expansion
state in the user interface.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 28 
 1 file changed, 28 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index d9d8a87..6751534 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -1,6 +1,22 @@
 
 from . import codegen
 
+
+# generate a new tree identifier
+ett_writer = None
+ett_num = 0
+def new_ett(writer):
+global ett_writer
+global ett_num
+
+if ett_num and ett_num % 16 == 0:
+ett_writer.newline()
+name = 'etts[%u]' % ett_num
+ett_num = ett_num + 1
+ett_writer.write('-1, ')
+return name
+
+
 hf_writer = None
 hf_defs = None
 
@@ -42,6 +58,10 @@ def write_protocol_definitions(writer):
 writer.newline()
 writer.function('spice_register_fields', 'void', 'int proto, 
expert_module_t* expert_proto')
 
+writer.variable_def('guint', 'n');
+writer.variable_def('gint *', 'ett[array_length(etts)]')
+writer.newline()
+
 writer.write(static hf_register_info hf[] = )
 writer.begin_block()
 hf_defs = writer.get_subwriter()
@@ -55,17 +75,25 @@ def write_protocol_definitions(writer):
 writer.end_block(semicolon = True)
 writer.newline()
 
+with writer.for_loop('n', 'array_length(etts)'):
+writer.assign('ett[n]', 'etts[n]')
+
 writer.statement('proto_register_field_array(proto, hf, array_length(hf))')
+writer.statement('proto_register_subtree_array(ett, array_length(etts))')
 writer.statement('expert_register_field_array(expert_proto, ei, 
array_length(ei))')
 writer.end_block()
 
 
 def write_protocol_parser(writer, proto):
 global hf_writer
+global ett_writer
 
 write_parser_helpers(writer)
 
 # put fields declaration first
+with writer.block('static gint etts[] =', semicolon=True) as scope:
+ett_writer = scope
+writer.newline()
 hf_writer = writer.get_subwriter()
 
 # put fields definition at last
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 28/33] Handle pointers

2015-07-01 Thread Frediano Ziglio
Read the pointer and if not 0 create a function to parse it and
call it.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 49 +
 1 file changed, 49 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 3e7af08..db78a82 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -280,9 +280,58 @@ def write_array(writer, container, member, nelements, 
array, dest, scope):
 assert(element_type.is_struct())
 write_struct(writer, member, element_type, index, dest, scope)
 
+
+def write_ptr_function(writer, target_type, container, member, dest, scope):
+nelements = ''
+if target_type.is_array():
+parse_function = parse_array_%s % 
target_type.element_type.primitive_type()
+nelements = ', guint32 nelements'
+else:
+parse_function = parse_struct_%s % target_type.c_type()
+if writer.is_generated(parser, parse_function):
+return '%s(glb, tree%d, offset)' % (parse_function, dest.level)
+
+writer.set_is_generated(parser, parse_function)
+
+writer = writer.function_helper()
+scope = writer.function(parse_function, guint32, GlobalInfo *glb _U_, 
proto_tree *tree0 _U_%s, guint32 offset % nelements, True)
+
+writer.newline()
+
+dest = RootDestination(scope)
+dest.is_helper = True
+if target_type.is_array():
+write_array(writer, None, None, nelements, target_type, dest, scope)
+else:
+write_container_parser(writer, target_type, dest)
+
+writer.statement(return offset)
+
+writer.end_block()
+
+return '%s(glb, tree%d, offset)' % (parse_function, dest.level)
+
+def read_ptr(writer, t):
+writer.assign('ptr', '%s(glb-tvb, offset)' % primitive_read_func(t))
+writer.increment('offset', str(t.get_fixed_nw_size()))
+
 def write_pointer(writer, container, member, t, dest, scope):
 assert(t.is_pointer())
 
+if not scope.variable_defined('ptr'):
+scope.variable_def('guint32', 'ptr')
+read_ptr(writer, t)
+with writer.if_block('ptr'):
+writer.variable_def('guint32', 'save_offset = offset')
+writer.assign('offset', 'ptr + glb-message_offset')
+if t.target_type.is_array():
+nelements = read_array_len(writer, member.name, t.target_type, 
dest, scope, True)
+write_array(writer, container, member, nelements, t.target_type, 
dest, scope)
+else:
+stmt = write_ptr_function(writer, t.target_type, container, 
member, dest, scope)
+writer.statement(stmt)
+writer.assign('offset', 'save_offset')
+
 
 def write_struct_func(writer, t, func_name, index):
 func_name = 'dissect_spice_struct_' + t.name
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 25/33] Handle array

2015-07-01 Thread Frediano Ziglio
If just data increment the pointer if not handle the items with
a loop.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index c9331cd..6e9f0d6 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -227,6 +227,20 @@ def write_switch(writer, container, switch, dest, scope):
 def write_array(writer, container, member, nelements, array, dest, scope):
 assert(container and member)
 
+element_type = array.element_type
+
+if element_type == ptypes.uint8 or element_type == ptypes.int8:
+writer.increment(offset, nelements)
+return
+
+with writer.index() as index:
+with writer.for_loop(index, nelements) as array_scope:
+if element_type.is_primitive():
+write_member_primitive(writer, container, member, 
element_type, dest, scope)
+else:
+assert(element_type.is_struct())
+write_struct(writer, member, element_type, index, dest, scope)
+
 def write_pointer(writer, container, member, t, dest, scope):
 assert(t.is_pointer())
 
@@ -245,7 +259,6 @@ def write_struct_func(writer, t, func_name, index):
 writer.statement('return offset')
 writer.end_block()
 
-
 def write_struct(writer, member, t, index, dest, scope):
 assert(t.is_struct())
 
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 29/33] Handle base fields

2015-07-01 Thread Frediano Ziglio
Add fields to base tree (so basically there is no tree).
Names is now generated from container + member name.
The check for duplicate is not that strong, should check if same field
is defined with same attributes.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 68 +
 1 file changed, 68 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index db78a82..23bb96a 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -175,6 +175,72 @@ def primitive_read_func(t):
 return 'tvb_get_letoh64'
 raise NotImplementedError('primitive size not supported %s %s' % (size, t))
 
+def container_hf_name(container):
+if container.has_name():
+hf_name = hf_%s % container.name
+else:
+hf_name = hf_%s % container.c_name()
+return hf_name
+
+def member_hf_name(container, member):
+if member.member_type.is_array():
+hf_name = %s_array_%s % (container_hf_name(container), member.name)
+else:
+hf_name = %s_%s % (container_hf_name(container), member.name)
+return hf_name
+
+def get_primitive_ft_type(t):
+assert(t.is_primitive())
+
+unsigned = 'U'
+if isinstance(t, ptypes.IntegerType) and t.signed:
+unsigned = ''
+size = t.get_fixed_nw_size()
+assert size in (1, 2, 4, 8)
+return FT_%sINT%d % (unsigned, size * 8)
+
+# write a field
+def write_wireshark_field(writer, container, member, t, tree, size, 
encoding='ENC_LITTLE_ENDIAN', prefix=''):
+
+assert(member and container)
+
+hf_name = member_hf_name(container, member)
+
+# compute proper type
+f_type = 'FT_NONE'
+base = 'BASE_NONE'
+vals = 'NULL'
+if encoding == 'ENC_LITTLE_ENDIAN':
+assert(t.is_primitive())
+base = 'BASE_DEC'
+f_type = get_primitive_ft_type(t)
+if isinstance(t, ptypes.FlagsType):
+# show flag as hexadecimal for now
+base = 'BASE_HEX'
+assert(t.has_name())
+vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
+elif isinstance(t, ptypes.EnumType) or isinstance(t, 
ptypes.EnumBaseType):
+base = 'BASE_DEC'
+assert(t.has_name())
+vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
+
+desc = member.name
+ws_name = 'auto.' + hf_name[3:]
+
+writer.statement(%sproto_tree_add_item(%s, %s, glb-tvb, offset, %s, %s) 
%
+(prefix, tree, hf_name, size, encoding))
+
+# TODO handle better duplications
+if hf_writer.variable_defined(hf_name):
+return
+hf_writer.variable_def(static int, %s = -1 % hf_name)
+
+hf_defs.writeln('{ %s,' % hf_name)
+hf_defs.writeln('  { %s, spice2.%s,' % (desc, ws_name))
+hf_defs.writeln('%s, %s, %s, 0,' % (f_type, base, vals))
+hf_defs.writeln('NULL, HFILL }')
+hf_defs.writeln('},')
+
 
 # Note: during parsing, byte_size types have been converted to count during 
validation
 def read_array_len(writer, prefix, array, dest, scope, is_ptr):
@@ -269,6 +335,7 @@ def write_array(writer, container, member, nelements, 
array, dest, scope):
 element_type = array.element_type
 
 if element_type == ptypes.uint8 or element_type == ptypes.int8:
+write_wireshark_field(writer, container, member, array, 'tree%d' % 
dest.level, nelements, 'ENC_NA')
 writer.increment(offset, nelements)
 return
 
@@ -364,6 +431,7 @@ def write_member_primitive(writer, container, member, t, 
dest, scope):
 
 if member.has_attr(bytes_count):
 raise NotImplementedError(bytes_count not implemented)
+write_wireshark_field(writer, container, member, t, 'tree%d' % dest.level, 
t.get_fixed_nw_size())
 if member.has_attr(bytes_count):
 dest_var = member.attributes[bytes_count][0]
 else:
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 14/33] Generate scheleton for messages and channels

2015-07-01 Thread Frediano Ziglio
Messages are not generated as stub.
Code partially from demarshaller.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 138 +++-
 1 file changed, 137 insertions(+), 1 deletion(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 67d0fa7..9971140 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -1,4 +1,5 @@
 
+from . import ptypes
 from . import codegen
 
 import types
@@ -53,6 +54,124 @@ def write_parser_helpers(writer):
 writer.writeln('#endif')
 writer.newline()
 
+def write_msg_parser(writer, message, server):
+msg_name = message.c_name()
+function_name = dissect_spice_%s_%s % ('server' if server else 'client', 
msg_name)
+if writer.is_generated(msg_parser, function_name):
+return function_name
+writer.set_is_generated(msg_parser, function_name)
+
+msg_type = message.c_type()
+msg_sizeof = message.sizeof()
+
+writer.newline()
+writer.ifdef(message)
+parent_scope = writer.function(function_name,
+   guint32,
+   GlobalInfo *glb _U_, proto_tree *tree0 
_U_, guint32 offset, True)
+
+writer.statement(return offset)
+writer.end_block()
+
+writer.endif(message)
+
+return function_name
+
+def write_channel_parser(writer, channel, server):
+writer.newline()
+ids = {}
+if server:
+messages = channel.server_messages
+else:
+messages = channel.client_messages
+for m in messages:
+ids[m.value] = m
+
+ranges = []
+ids2 = ids.copy()
+while len(ids2)  0:
+end = start = min(ids2.keys())
+while end in ids2:
+del ids2[end]
+end = end + 1
+
+ranges.append( (start, end) )
+
+if server:
+function_name = dissect_spice_data_server_%s % channel.name
+else:
+function_name = dissect_spice_data_client_%s % channel.name
+writer.newline()
+writer.ifdef(channel)
+scope = writer.function(function_name,
+guint32,
+guint16 message_type, GlobalInfo *glb _U_, 
proto_tree *tree _U_, guint32 offset, True)
+
+helpers = writer.function_helper()
+
+d = 0
+for r in ranges:
+d = d + 1
+writer.write(static const parse_msg_func_t funcs%d[%d] =  % (d, r[1] 
- r[0]))
+writer.begin_block()
+for i in range(r[0], r[1]):
+func = write_msg_parser(helpers, ids[i].message_type, server)
+writer.write(func)
+if i != r[1] -1:
+writer.write(,)
+writer.newline()
+
+writer.end_block(semicolon = True)
+
+d = 0
+for r in ranges:
+d = d + 1
+with writer.if_block(message_type = %d  message_type  %d % 
(r[0], r[1]), d  1, False):
+writer.statement(return funcs%d[message_type-%d](glb, tree, 
offset) % (d, r[0]))
+writer.newline()
+
+writer.statement('expert_add_info_format(glb-pinfo, glb-msgtype_item, 
ei_spice_unknown_message, Unknown display server message - cannot dissect)')
+writer.statement(return offset)
+writer.end_block()
+writer.endif(channel)
+
+return function_name
+
+def write_get_channel_parser(writer, channel_parsers, max_channel, is_server):
+writer.newline()
+function_name = spice_%s%s_channel_get_dissect % ('server' if is_server 
else 'client', writer.public_prefix)
+
+writer.function(function_name, spice_dissect_func_t, guint8 channel)
+
+writer.write(static const spice_dissect_func_t channels[%d] =  % 
(max_channel+1))
+writer.begin_block()
+channel = None
+for i in range(0, max_channel + 1):
+if i in channel_parsers:
+channel = channel_parsers[i][0]
+writer.ifdef(channel)
+writer.write(channel_parsers[i][1])
+else:
+writer.write(NULL)
+
+if i != max_channel:
+writer.write(,)
+writer.newline()
+if channel and channel.has_attr(ifdef):
+writer.ifdef_else(channel)
+writer.write(NULL)
+if i != max_channel:
+writer.write(,)
+writer.newline()
+writer.endif(channel)
+writer.end_block(semicolon = True)
+
+with writer.if_block(channel  %d % (max_channel + 1)):
+writer.statement(return channels[channel])
+
+writer.statement(return NULL)
+writer.end_block()
+
 
 def write_protocol_definitions(writer):
 global hf_defs
@@ -114,8 +233,25 @@ def write_protocol_parser(writer, proto):
 writer.newline()
 hf_writer = writer.get_subwriter()
 
+# put everything else after
+defs_writer = writer
+writer = writer.get_subwriter()
+
 # put fields definition at last
-write_protocol_definitions(writer)
+write_protocol_definitions(defs_writer)
+
+# scan all, looks for items and tree
+ 

[Spice-devel] [PATCH 17/33] Decorate protocol file with attributes for wireshark

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 spice.proto | 414 
 1 file changed, 218 insertions(+), 196 deletions(-)

diff --git a/spice.proto b/spice.proto
index 2889802..0bcb421 100644
--- a/spice.proto
+++ b/spice.proto
@@ -5,14 +5,14 @@
 typedef fixed28_4 int32 @ctype(SPICE_FIXED28_4);
 
 struct Point {
-int32 x;
-int32 y;
-};
+int32 x @ws(x,point32.x);
+int32 y @ws(y,point32.y);
+} @ws_txt(POINT (%d, %d), x, y);
 
 struct Point16 {
-int16 x;
-int16 y;
-};
+int16 x @ws(x,point16.x);
+int16 y @ws(y,point16.y);
+} @ws_txt(POINT16 (%d, %d), x, y);
 
 struct PointFix {
 fixed28_4 x;
@@ -20,11 +20,14 @@ struct PointFix {
 };
 
 struct Rect {
-int32 top;
-int32 left;
-int32 bottom;
-int32 right;
-};
+int32 top @ws(top,rect.top);
+int32 left @ws(left,rect.left);
+int32 bottom @ws(bottom,rect.bottom);
+int32 right @ws(right,rect.right);
+}
+@ws_txt(RECT: (%d-%d, %d-%d), left, top, right, bottom)
+@ws_txt_n(RECT %u: (%d-%d, %d-%d), INDEX, left, top, right, bottom)
+;
 
 struct Transform {
 uint32 t00;
@@ -96,10 +99,15 @@ enum32 notify_visibility {
 };
 
 flags16 mouse_mode {
-SERVER,
-CLIENT,
+SERVER @ws_desc(Server mode),
+CLIENT @ws_desc(Client mode),
 };
 
+flags32 mouse_mode32 {
+SERVER @ws_desc(Server mode),
+CLIENT @ws_desc(Client mode),
+} @ws(Supported mouse modes, supported_mouse_modes);
+
 enum16 pubkey_type {
 INVALID,
 RSA,
@@ -117,12 +125,12 @@ message Empty {
 };
 
 message Data {
-uint8 data[] @end @ctype(uint8_t);
+uint8 data[] @end @ctype(uint8_t) @ws(data, data) @ws_type(BYTES);
 } @nocopy;
 
 struct ChannelWait {
 uint8 channel_type;
-uint8 channel_id;
+uint8 channel_id @ws_desc(Channel ID);
 uint64 message_serial;
 } @ctype(SpiceWaitForChannel);
 
@@ -135,14 +143,14 @@ channel BaseChannel {
 Data migrate_data;
 
 message {
-   uint32 generation;
-   uint32 window;
+   uint32 generation @ws(Set ACK generation,red_set_ack_generation);
+   uint32 window @ws(Set ACK window (messages),red_set_ack_window);
 } set_ack;
 
 message {
-   uint32 id;
-   uint64 timestamp;
-   uint8 data[] @ctype(uint8_t) @as_ptr(data_len);
+   uint32 id @ws(Ping ID,ping_id);
+   uint64 timestamp @ws(timestamp,timestamp);
+   uint8 data[] @ctype(uint8_t) @as_ptr(data_len) @ws_txt(PING DATA (%d 
bytes), data.nelements);
 } ping;
 
 message {
@@ -156,12 +164,12 @@ channel BaseChannel {
 } @ctype(SpiceMsgDisconnect) disconnecting;
 
 message {
-   uint64 time_stamp;
-   notify_severity severity;
-   notify_visibility visibilty;
-   uint32 what; /* error_code/warn_code/info_code */
-   uint32 message_len;
-   uint8 message[message_len] @end @nomarshal;
+   uint64 time_stamp @ws(timestamp,timestamp);
+   notify_severity severity @ws(Severity,notify_severity);
+   notify_visibility visibilty  @ws(Visibility,notify_visibility);
+   uint32 what @ws(error/warn/info code, notify_code); /* 
error_code/warn_code/info_code */
+   uint32 message_len @ws(Message length,notify_message_length);
+   uint8 message[message_len] @end @nomarshal 
@ws(Message,notify_message) @ws_type(STRING);
 } notify;
 
 Data list; /* the msg body is SpiceSubMessageList */
@@ -170,14 +178,14 @@ channel BaseChannel {
 
  client:
 message {
-   uint32 generation;
+   uint32 generation  @ws(Set ACK generation,red_set_ack_generation);
 } ack_sync;
 
 Empty ack;
 
 message {
-   uint32 id;
-   uint64 timestamp;
+   uint32 id @ws(Ping ID,ping_id);
+   uint64 timestamp @ws(timestamp,timestamp);
 } @ctype(SpiceMsgPing) pong;
 
 Empty migrate_flush_mark;
@@ -191,17 +199,17 @@ channel BaseChannel {
 };
 
 struct ChannelId {
-uint8 type;
-uint8 id;
-};
+uint8 type @ws(Channel type,channel_type) @ws_type(CHANNEL);
+uint8 id @ws(Channel ID, channel_id);
+} @ws_txt_n(channels[%u], INDEX);
 
 struct DstInfo {
-   uint16 port;
-   uint16 sport;
+   uint16 port @ws(Migrate Dest Port,migrate_dest_port);
+   uint16 sport @ws(Migrate Dest Secure Port,migrate_dest_sport);
uint32 host_size;
-   uint8 *host_data[host_size] @zero_terminated @marshall @nonnull;
+   uint8 *host_data[host_size] @zero_terminated @marshall @nonnull 
@ws(data, data) @ws_type(BYTES);
uint32 cert_subject_size;
-   uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
+   uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall 
@ws(data, data) @ws_type(BYTES);
 } @ctype(SpiceMigrationDstInfo);
 
 channel MainChannel : BaseChannel {
@@ -213,69 +221,69 @@ channel MainChannel : BaseChannel {
 Empty migrate_cancel;
 
 message {
-   uint32 session_id;
-   uint32 display_channels_hint;
-   uint32 supported_mouse_modes;
-   

[Spice-devel] [PATCH 27/33] Implement ws_inline attribute

2015-07-01 Thread Frediano Ziglio
This attribute allow structure to be aligned instead of be contained
in a separate function.
This is helpful as variable are declared in the function so allows
other member to reference to a nested structure.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index f18b5b4..3e7af08 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -301,9 +301,14 @@ def write_struct_func(writer, t, func_name, index):
 def write_struct(writer, member, t, index, dest, scope):
 assert(t.is_struct())
 
-func_name = 'dissect_spice_struct_' + t.name
-write_struct_func(writer, t, func_name, index)
-writer.assign('offset', '%s(glb, tree%d, offset, %s)' % (func_name, 
dest.level, index))
+if member.has_attr('ws_inline'):
+dest = dest.child_sub(member.name, scope)
+with writer.block() as scope:
+write_container_parser(writer, t, dest)
+else:
+func_name = 'dissect_spice_struct_' + t.name
+write_struct_func(writer, t, func_name, index)
+writer.assign('offset', '%s(glb, tree%d, offset, %s)' % (func_name, 
dest.level, index))
 
 def write_member_primitive(writer, container, member, t, dest, scope):
 assert(t.is_primitive())
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 23/33] Read array size

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 50 +++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index cb07ab6..4db2f3c 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -176,10 +176,55 @@ def primitive_read_func(t):
 raise NotImplementedError('primitive size not supported %s %s' % (size, t))
 
 
+# Note: during parsing, byte_size types have been converted to count during 
validation
+def read_array_len(writer, prefix, array, dest, scope, is_ptr):
+if is_ptr:
+nelements = %s__array__nelements % prefix
+else:
+nelements = %s__nelements % prefix
+if dest.is_toplevel() and scope.variable_defined(nelements):
+return nelements # Already there for toplevel, need not recalculate
+# just reuse variable, there is no problem for cast as we always store in 
guint32
+if array.is_identifier_length():
+nelements = dest.read_ref(array.size)
+dest.write_ref(writer, dest.ref_size(array.size), prefix + 
'.nelements', nelements)
+return nelements
+element_type = array.element_type
+scope.variable_def(guint32, nelements)
+if array.is_constant_length():
+writer.assign(nelements, array.size)
+elif array.is_remaining_length():
+if element_type.is_fixed_nw_size():
+if element_type.get_fixed_nw_size() == 1:
+writer.assign(nelements, glb-message_end - offset)
+else:
+writer.assign(nelements, (glb-message_end - offset) / (%s) 
%(element_type.get_fixed_nw_size()))
+else:
+raise NotImplementedError(TODO array[] of dynamic element size 
not done yet)
+elif array.is_image_size_length():
+bpp = array.size[1]
+width = array.size[2]
+rows = array.size[3]
+width_v = dest.read_ref(width)
+rows_v = dest.read_ref(rows)
+if bpp == 8:
+writer.assign(nelements, ((guint32) %s * %s) % (width_v, rows_v))
+elif bpp == 1:
+writer.assign(nelements, (((guint32) %s + 7U) / 8U ) * %s % 
(width_v, rows_v))
+else:
+writer.assign(nelements, ((%sU * (guint32) %s + 7U) / 8U ) * %s 
% (bpp, width_v, rows_v))
+elif array.is_bytes_length():
+writer.assign(nelements, dest.read_ref(array.size[2]))
+else:
+raise NotImplementedError(TODO array size type not handled yet)
+# TODO compute a better size
+dest.write_ref(writer, 32, prefix+'.nelements', nelements)
+return nelements
+
 def write_switch(writer, container, switch, dest, scope):
 pass
 
-def write_array(writer, container, member, array, dest, scope):
+def write_array(writer, container, member, nelements, array, dest, scope):
 assert(container and member)
 
 def write_pointer(writer, container, member, t, dest, scope):
@@ -227,7 +272,8 @@ def write_member(writer, container, member, dest, scope):
 elif t.is_primitive():
 write_member_primitive(writer, container, member, t, dest, scope)
 elif t.is_array():
-write_array(writer, container, member, t, dest, scope)
+nelements = read_array_len(writer, member.name, t, dest, scope, False)
+write_array(writer, container, member, nelements, t, dest, scope)
 
 elif t.is_struct():
 write_struct(writer, member, t, '-1', dest, scope)
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 32/33] Use a class to register wireshark fields

2015-07-01 Thread Frediano Ziglio
Allow to reuse code and check better if field was already registered

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 59 +
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index edb8533..7070c3a 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -20,8 +20,50 @@ def new_ett(writer):
 return name
 
 
+# handle registration of wireshark flags
 hf_writer = None
 hf_defs = None
+class HF:
+def __init__(self, hf_name, desc=''):
+self.hf_name = hf_name
+self.desc = desc
+self.mask = '0'
+
+def __str__(self):
+x = []
+for f in 'hf_name desc ws_name f_type base vals mask'.split():
+x.append(f + ':' + self.__dict__[f])
+return ' '.join(x)
+
+def debug(self, writer, s):
+raise Exception(s)
+
+# declare a field identifier
+def add_wireshark_field(self):
+if hf_writer.variable_defined(self.hf_name):
+raise Exception('HF field %s already defined' % self.hf_name)
+hf_writer.variable_def(static int, %s = -1 % self.hf_name)
+
+def create(self, writer):
+other = hf_defs.fields.get(self.ws_name)
+if other:
+for f in 'hf_name desc ws_name base vals mask'.split():
+if other.__dict__[f] != self.__dict__[f]:
+self.debug(writer, 'HF Field different from previous 
for\n\t%s\n\t%s' % (other, self))
+if other.f_type != self.f_type:
+if other.f_type[:7] != 'FT_UINT' or self.f_type[:7] != 
'FT_UINT':
+self.debug(writer, 'HF Field different from previous 
for\n\t%s\n\t%s' % (other, self))
+return
+
+hf_defs.fields[self.ws_name] = self
+
+self.add_wireshark_field()
+
+hf_defs.writeln('{ %s,' % self.hf_name)
+hf_defs.writeln('  { %s, spice2.%s,' % (self.desc, self.ws_name))
+hf_defs.writeln('%s, %s, %s, %s,' % (self.f_type, self.base, 
self.vals, self.mask))
+hf_defs.writeln('NULL, HFILL }')
+hf_defs.writeln('},')
 
 
 def write_parser_helpers(writer):
@@ -274,16 +316,13 @@ def write_wireshark_field(writer, container, member, t, 
tree, size, encoding='EN
 writer.statement(%sproto_tree_add_item(%s, %s, glb-tvb, offset, %s, %s) 
%
 (prefix, tree, hf_name, size, encoding))
 
-# TODO handle better duplications
-if hf_writer.variable_defined(hf_name):
-return
-hf_writer.variable_def(static int, %s = -1 % hf_name)
-
-hf_defs.writeln('{ %s,' % hf_name)
-hf_defs.writeln('  { %s, spice2.%s,' % (desc, ws_name))
-hf_defs.writeln('%s, %s, %s, 0,' % (f_type, base, vals))
-hf_defs.writeln('NULL, HFILL }')
-hf_defs.writeln('},')
+# write definition
+hf = HF(hf_name, desc)
+hf.ws_name = ws_name
+hf.f_type = f_type
+hf.base = base
+hf.vals = vals
+hf.create(writer)
 
 
 # Note: during parsing, byte_size types have been converted to count during 
validation
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 30/33] Allow to override default values generated for the fields

2015-07-01 Thread Frediano Ziglio
ws_type override the type (BOOLEAN - FT_BOOLEAN).
ws_base override the base (DEC - BASE_HEX).
ws_desc override the description.
ws allow to specify description and name for wireshark. Name is
important as allows filters. Having a single attribute with 2 values
allows to quickly specify the main attributes.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 46 ++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 23bb96a..5b14b92 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -199,12 +199,30 @@ def get_primitive_ft_type(t):
 assert size in (1, 2, 4, 8)
 return FT_%sINT%d % (unsigned, size * 8)
 
+# fix some attributes
+# ws attribute contains description and optionally a name
+def fix_attributes(t):
+if t and t.has_attr('ws'):
+ws = t.attributes['ws']
+if not t.has_attr('ws_name'):
+t.attributes['ws_name'] = [ws[1]]
+if not t.has_attr('ws_desc'):
+t.attributes['ws_desc'] = [ws[0]]
+
 # write a field
 def write_wireshark_field(writer, container, member, t, tree, size, 
encoding='ENC_LITTLE_ENDIAN', prefix=''):
 
 assert(member and container)
 
-hf_name = member_hf_name(container, member)
+def get_member_t_attr(attr, default=None):
+if t.has_attr(attr):
+default = t.attributes[attr][0]
+if member and member.has_attr(attr):
+default = member.attributes[attr][0]
+return default
+
+fix_attributes(member)
+fix_attributes(t)
 
 # compute proper type
 f_type = 'FT_NONE'
@@ -224,8 +242,30 @@ def write_wireshark_field(writer, container, member, t, 
tree, size, encoding='EN
 assert(t.has_name())
 vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
 
-desc = member.name
-ws_name = 'auto.' + hf_name[3:]
+# override type
+ws_type = get_member_t_attr('ws_type')
+if ws_type:
+f_type = 'FT_%s' % ws_type
+if f_type == 'FT_BOOLEAN':
+vals = 'TFS(tfs_set_notset)'
+base = 'BASE_NONE'
+vals = 'NULL'
+
+# override base
+ws_base = get_member_t_attr('ws_base')
+if ws_base:
+base = 'BASE_%s' % ws_base
+
+# read description
+desc = get_member_t_attr('ws_desc', member.name)
+
+# read name
+ws_name = get_member_t_attr('ws_name')
+if not ws_name:
+hf_name = member_hf_name(container, member)
+ws_name = 'auto.' + hf_name[3:]
+else:
+hf_name = 'hf_%s' % ws_name.replace('.', '_')
 
 writer.statement(%sproto_tree_add_item(%s, %s, glb-tvb, offset, %s, %s) 
%
 (prefix, tree, hf_name, size, encoding))
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 33/33] Handle flags

2015-07-01 Thread Frediano Ziglio
Instead of only show the hexadecimal value show all bits.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 77 ++---
 1 file changed, 73 insertions(+), 4 deletions(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 7070c3a..8a15fd1 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -275,10 +275,15 @@ def write_wireshark_field(writer, container, member, t, 
tree, size, encoding='EN
 base = 'BASE_DEC'
 f_type = get_primitive_ft_type(t)
 if isinstance(t, ptypes.FlagsType):
-# show flag as hexadecimal for now
-base = 'BASE_HEX'
-assert(t.has_name())
-vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
+# if the attribute unique_flag is not set must compute
+# all flags writing a HF for each bit
+if t.has_attr('unique_flag'):
+base = 'BASE_HEX'
+assert(t.has_name())
+vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
+else:
+write_flags(writer, member, t, tree)
+return
 elif isinstance(t, ptypes.EnumType) or isinstance(t, 
ptypes.EnumBaseType):
 base = 'BASE_DEC'
 assert(t.has_name())
@@ -509,6 +514,70 @@ def write_struct(writer, member, t, index, dest, scope):
 write_struct_func(writer, t, func_name, index)
 writer.assign('offset', '%s(glb, tree%d, offset, %s)' % (func_name, 
dest.level, index))
 
+
+def write_flags_func(writer, t, hf_name):
+func_name = 'dissect_flags_' + t.name
+
+if writer.is_generated(flags, t.name):
+return func_name
+writer.set_is_generated(flags, t.name)
+
+writer = writer.function_helper()
+scope = writer.function(func_name, void, GlobalInfo *glb _U_, 
proto_tree *tree _U_, guint32 offset, int hf, True)
+dest = RootDestination(scope)
+
+size = t.get_fixed_nw_size()
+
+fix_attributes(t)
+
+desc = t.attributes.get('ws_desc', [t.name])[0] # TODO how to handle 
member ??
+hf = HF(hf_name, desc)
+hf.ws_name = '%s_flags' % (t.name.lower())
+hf.f_type = get_primitive_ft_type(t)
+hf.base = 'BASE_HEX' # TODO
+hf.vals = 'NULL'
+hf.create(writer)
+
+with writer.if_block('hf = 0'):
+writer.variable_def('proto_item *', 'ti')
+writer.newline()
+writer.assign('ti', 'proto_tree_add_item(tree, hf, glb-tvb, offset, 
%d, ENC_LITTLE_ENDIAN)' % size)
+writer.assign('tree', 'proto_item_add_subtree(ti, %s)' % 
new_ett(writer))
+
+values = list(t.names.keys())
+values.sort()
+values.reverse()
+bits = max(values) + 1
+for v in values:
+name = hf_name + '_' + t.names[v].lower()
+
+desc = t.descs[v] if t.descs[v] else t.names[v]
+hf = HF(name, desc)
+hf.ws_name = '%s_%s' % (t.name, t.names[v].lower())
+hf.f_type = 'FT_BOOLEAN'
+hf.base = str(bits)
+hf.vals = 'TFS(tfs_set_notset)'
+hf.mask = t.c_enumname(v)
+hf.create(writer)
+
+writer.statement('proto_tree_add_item(tree, %s, glb-tvb, offset, %d, 
ENC_LITTLE_ENDIAN)' % (name, size))
+
+writer.end_block()
+
+return func_name
+
+
+def write_flags(writer, member, t, tree):
+# TODO check size
+
+# TODO if some txt are defined as member use another item, not default ones
+
+# write a function to dissect the type
+hf_name = 'hf_%s_flag' % t.name
+fname = write_flags_func(writer, t, hf_name)
+writer.statement('%s(glb, %s, offset, %s)' % (fname, tree, hf_name))
+
+
 def write_member_primitive(writer, container, member, t, dest, scope):
 assert(t.is_primitive())
 
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 11/33] Generate some definition for dissector

2015-07-01 Thread Frediano Ziglio
Generate global definitions.
Generate function to registers various dissector components.
For the moment the field array is empty bu we save some global to
be able to register new ones.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 69 -
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 90ba498..d9d8a87 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -1,8 +1,75 @@
 
 from . import codegen
 
+hf_writer = None
+hf_defs = None
+
+
+def write_parser_helpers(writer):
+if writer.is_generated(helper, demarshaller):
+return
+
+writer.set_is_generated(helper, demarshaller)
+
+writer = writer.function_helper()
+
+writer.begin_block(typedef struct GlobalInfo)
+writer.variable_def(tvbuff_t *, tvb)
+writer.variable_def(packet_info *, pinfo)
+writer.variable_def(proto_item *, msgtype_item)
+writer.variable_def(guint32, message_offset)
+writer.variable_def(guint32, message_end)
+writer.end_block(newline=False)
+writer.writeln(' GlobalInfo;')
+
+writer.newline()
+writer.statement('static expert_field ei_spice_unknown_message = EI_INIT')
+
+writer.newline()
+writer.statement('typedef guint32 (*parse_msg_func_t)(GlobalInfo *glb, 
proto_tree *tree, guint32 offset)')
+writer.statement('typedef guint32 (*spice_dissect_func_t)(guint16 
message_type, GlobalInfo *glb, proto_tree *tree, guint32 offset)')
+
+writer.newline()
+writer.writeln('#ifndef _U_')
+writer.writeln('#define _U_')
+writer.writeln('#endif')
+writer.newline()
+
+
+def write_protocol_definitions(writer):
+global hf_defs
+
+writer.newline()
+writer.function('spice_register_fields', 'void', 'int proto, 
expert_module_t* expert_proto')
+
+writer.write(static hf_register_info hf[] = )
+writer.begin_block()
+hf_defs = writer.get_subwriter()
+hf_defs.fields = {}
+writer.end_block(semicolon = True)
+writer.newline()
+
+writer.write('static ei_register_info ei[] =')
+writer.begin_block()
+writer.writeln('{ ei_spice_unknown_message, { spice.unknown_message, 
PI_UNDECODED, PI_WARN, Unknown message - cannot dissect, EXPFILL }},')
+writer.end_block(semicolon = True)
+writer.newline()
+
+writer.statement('proto_register_field_array(proto, hf, array_length(hf))')
+writer.statement('expert_register_field_array(expert_proto, ei, 
array_length(ei))')
+writer.end_block()
+
+
 def write_protocol_parser(writer, proto):
-pass
+global hf_writer
+
+write_parser_helpers(writer)
+
+# put fields declaration first
+hf_writer = writer.get_subwriter()
+
+# put fields definition at last
+write_protocol_definitions(writer)
 
 def write_includes(writer):
 writer.newline()
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 24/33] Generate code to output parse structure

2015-07-01 Thread Frediano Ziglio
Write a function and call it to be able to reuse it.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 20 
 1 file changed, 20 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 4db2f3c..c9331cd 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -230,9 +230,29 @@ def write_array(writer, container, member, nelements, 
array, dest, scope):
 def write_pointer(writer, container, member, t, dest, scope):
 assert(t.is_pointer())
 
+
+def write_struct_func(writer, t, func_name, index):
+func_name = 'dissect_spice_struct_' + t.name
+
+if writer.is_generated(struct, t.name):
+return
+writer.set_is_generated(struct, t.name)
+
+writer = writer.function_helper()
+scope = writer.function(func_name, guint32, GlobalInfo *glb _U_, 
proto_tree *tree0 _U_, guint32 offset, gint32 index _U_, True)
+dest = RootDestination(scope)
+write_container_parser(writer, t, dest)
+writer.statement('return offset')
+writer.end_block()
+
+
 def write_struct(writer, member, t, index, dest, scope):
 assert(t.is_struct())
 
+func_name = 'dissect_spice_struct_' + t.name
+write_struct_func(writer, t, func_name, index)
+writer.assign('offset', '%s(glb, tree%d, offset, %s)' % (func_name, 
dest.level, index))
+
 def write_member_primitive(writer, container, member, t, dest, scope):
 assert(t.is_primitive())
 
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 04/33] codegen: Optimize code indentation avoiding loop

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/codegen.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/python_modules/codegen.py b/python_modules/codegen.py
index 55500b7..02ffdb9 100644
--- a/python_modules/codegen.py
+++ b/python_modules/codegen.py
@@ -130,8 +130,7 @@ class CodeWriter:
 return
 
 if self.at_line_start:
-for i in range(self.indentation):
-self.out.write(u )
+self.out.write(u  * self.indentation)
 self.at_line_start = False
 self.out.write(s)
 return self
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 26/33] Handle switch

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 41 -
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 6e9f0d6..f18b5b4 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -221,8 +221,47 @@ def read_array_len(writer, prefix, array, dest, scope, 
is_ptr):
 dest.write_ref(writer, 32, prefix+'.nelements', nelements)
 return nelements
 
+
 def write_switch(writer, container, switch, dest, scope):
-pass
+var = container.lookup_member(switch.variable)
+var_type = var.member_type
+
+if switch.has_attr(fixedsize):
+scope.variable_def(guint32, save_output)
+writer.assign(save_output, output)
+
+first = True
+for c in switch.cases:
+check = c.get_check(dest.read_ref(switch.variable), var_type)
+m = c.member
+with writer.if_block(check, not first, False) as block:
+t = m.member_type
+if switch.has_attr(anon):
+dest2 = dest
+else:
+if t.is_struct():
+dest2 = dest.child_sub(switch.name + . + m.name, block)
+else:
+dest2 = dest.child_sub(switch.name, block)
+
+if t.is_pointer():
+write_pointer(writer, container, m, t, dest2, block)
+elif t.is_struct():
+write_struct(writer, m, t, '-1', dest2, scope)
+elif t.is_primitive():
+write_member_primitive(writer, container, m, t, dest2, scope)
+elif t.is_array():
+nelements = read_array_len(writer, m.name, t, dest, block, 
False)
+write_array(writer, container, m, nelements, t, dest2, block)
+else:
+writer.todo(Can't handle type %s % m.member_type)
+
+first = False
+
+writer.newline()
+
+if switch.has_attr(fixedsize):
+writer.assign(output, save_output + %s % 
switch.get_fixed_nw_size())
 
 def write_array(writer, container, member, nelements, array, dest, scope):
 assert(container and member)
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 31/33] Allow to specify 'CHANNEL' as type

2015-07-01 Thread Frediano Ziglio
Type will be mapped to an enumerator containing all channel types.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 5b14b92..edb8533 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -245,11 +245,15 @@ def write_wireshark_field(writer, container, member, t, 
tree, size, encoding='EN
 # override type
 ws_type = get_member_t_attr('ws_type')
 if ws_type:
-f_type = 'FT_%s' % ws_type
-if f_type == 'FT_BOOLEAN':
-vals = 'TFS(tfs_set_notset)'
-base = 'BASE_NONE'
-vals = 'NULL'
+if ws_type == 'CHANNEL':
+base = 'BASE_DEC'
+vals = 'VALS(channel_types_vs)'
+else:
+f_type = 'FT_%s' % ws_type
+if f_type == 'FT_BOOLEAN':
+vals = 'TFS(tfs_set_notset)'
+base = 'BASE_NONE'
+vals = 'NULL'
 
 # override base
 ws_base = get_member_t_attr('ws_base')
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 20/33] Parse containers

2015-07-01 Thread Frediano Ziglio
Parse all members of the containers

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index bd68262b..47d4031 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -153,6 +153,24 @@ class SubDestination(Destination):
 return self.parent_dest.get_ref(self.member + . + member, writer)
 
 
+def write_member_parser(writer, container, member, dest, scope):
+pass
+
+def write_container_parser(writer, container, dest):
+if container.has_attr('ws_as'):
+type_name = container.attributes['ws_as'][0]
+assert(ptypes.type_exists(type_name))
+container = ptypes.lookup_type(type_name)
+
+with dest.declare(writer) as scope:
+for m in container.members:
+if m.has_minor_attr():
+writer.begin_block(if (minor = %s) % m.get_minor_attr())
+write_member_parser(writer, container, m, dest, scope)
+if m.has_minor_attr():
+writer.end_block()
+
+
 def write_msg_parser(writer, message, server):
 msg_name = message.c_name()
 function_name = dissect_spice_%s_%s % ('server' if server else 'client', 
msg_name)
@@ -170,6 +188,7 @@ def write_msg_parser(writer, message, server):
GlobalInfo *glb _U_, proto_tree *tree0 
_U_, guint32 offset, True)
 
 dest = RootDestination(parent_scope)
+write_container_parser(writer, message, dest)
 
 writer.statement(return offset)
 writer.end_block()
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 22/33] Read values from primitive fields

2015-07-01 Thread Frediano Ziglio
Store into references for future usage.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 32 
 1 file changed, 32 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 707eae9..cb07ab6 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -153,6 +153,29 @@ class SubDestination(Destination):
 return self.parent_dest.get_ref(self.member + . + member, writer)
 
 
+def primitive_read_func(t):
+assert(t.is_primitive())
+
+signed = False
+if isinstance(t, ptypes.IntegerType) and t.signed:
+signed = True
+
+size = t.get_fixed_nw_size()
+if size == 1:
+if signed:
+return '(gint8) tvb_get_guint8'
+return 'tvb_get_guint8'
+elif size == 2:
+if signed:
+return '(gint32) (gint16) tvb_get_letohs'
+return 'tvb_get_letohs'
+elif size == 4:
+return 'tvb_get_letohl'
+elif size == 8:
+return 'tvb_get_letoh64'
+raise NotImplementedError('primitive size not supported %s %s' % (size, t))
+
+
 def write_switch(writer, container, switch, dest, scope):
 pass
 
@@ -168,6 +191,15 @@ def write_struct(writer, member, t, index, dest, scope):
 def write_member_primitive(writer, container, member, t, dest, scope):
 assert(t.is_primitive())
 
+if member.has_attr(bytes_count):
+raise NotImplementedError(bytes_count not implemented)
+if member.has_attr(bytes_count):
+dest_var = member.attributes[bytes_count][0]
+else:
+dest_var = member.name
+dest.write_ref(writer, t.get_fixed_nw_size() * 8, dest_var, '%s(glb-tvb, 
offset)' % primitive_read_func(t))
+writer.increment(offset, t.get_fixed_nw_size())
+
 def write_member(writer, container, member, dest, scope):
 
 if member.has_attr(virtual):
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 21/33] Write function to write members

2015-07-01 Thread Frediano Ziglio
Check members are all of a giver type and call stubs for each type.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 50 +++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 47d4031..707eae9 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -153,9 +153,55 @@ class SubDestination(Destination):
 return self.parent_dest.get_ref(self.member + . + member, writer)
 
 
-def write_member_parser(writer, container, member, dest, scope):
+def write_switch(writer, container, switch, dest, scope):
 pass
 
+def write_array(writer, container, member, array, dest, scope):
+assert(container and member)
+
+def write_pointer(writer, container, member, t, dest, scope):
+assert(t.is_pointer())
+
+def write_struct(writer, member, t, index, dest, scope):
+assert(t.is_struct())
+
+def write_member_primitive(writer, container, member, t, dest, scope):
+assert(t.is_primitive())
+
+def write_member(writer, container, member, dest, scope):
+
+if member.has_attr(virtual):
+dest.write_ref(writer, 32, member.name, 
member.attributes[virtual][0])
+return
+
+writer.comment(member.name)
+writer.newline()
+
+if member.is_switch():
+write_switch(writer, container, member, dest, scope)
+return
+
+if member.has_attr('ws_as'):
+type_name = member.attributes['ws_as'][0]
+assert(ptypes.type_exists(type_name))
+t = ptypes.lookup_type(type_name)
+else:
+t = member.member_type
+
+if t.is_pointer():
+# TODO case not handled
+if not member.has_attr(nocopy):
+write_pointer(writer, container, member, t, dest, scope)
+elif t.is_primitive():
+write_member_primitive(writer, container, member, t, dest, scope)
+elif t.is_array():
+write_array(writer, container, member, t, dest, scope)
+
+elif t.is_struct():
+write_struct(writer, member, t, '-1', dest, scope)
+else:
+raise NotImplementedError(TODO can't handle parsing of %s % t)
+
 def write_container_parser(writer, container, dest):
 if container.has_attr('ws_as'):
 type_name = container.attributes['ws_as'][0]
@@ -166,7 +212,7 @@ def write_container_parser(writer, container, dest):
 for m in container.members:
 if m.has_minor_attr():
 writer.begin_block(if (minor = %s) % m.get_minor_attr())
-write_member_parser(writer, container, m, dest, scope)
+write_member(writer, container, m, dest, scope)
 if m.has_minor_attr():
 writer.end_block()
 
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 06/33] codegen: Reuse code to fix attribute from prototype file

2015-07-01 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/ptypes.py | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index d031d09..845fa73 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -62,6 +62,14 @@ class FixedSize:
 # other members
 propagated_attributes=[ptr_array, nonnull, chunk]
 
+def fix_attributes(attribute_list):
+attrs = {}
+for attr in attribute_list:
+name = attr[0][1:]
+lst = attr[1:]
+attrs[name] = lst
+return attrs
+
 class Type:
 def __init__(self):
 self.attributes = {}
@@ -178,8 +186,7 @@ class TypeAlias(Type):
 Type.__init__(self)
 self.name = name
 self.the_type = the_type
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def get_type(self, recursive=False):
 if recursive:
@@ -288,8 +295,7 @@ class EnumType(EnumBaseType):
 self.names = names
 self.values = values
 
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def __str__(self):
 return enum %s % self.name
@@ -342,8 +348,7 @@ class FlagsType(EnumBaseType):
 self.names = names
 self.values = values
 
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def __str__(self):
 return flags %s % self.name
@@ -533,8 +538,7 @@ class Member(Containee):
 Containee.__init__(self)
 self.name = name
 self.member_type = member_type
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def resolve(self, container):
 self.container = container
@@ -636,8 +640,7 @@ class Switch(Containee):
 self.variable = variable
 self.name = name
 self.cases = cases
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def is_switch(self):
 return True
@@ -846,8 +849,7 @@ class StructType(ContainerType):
 self.members_by_name = {}
 for m in members:
 self.members_by_name[m.name] = m
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def __str__(self):
 if self.name == None:
@@ -869,8 +871,7 @@ class MessageType(ContainerType):
 for m in members:
 self.members_by_name[m.name] = m
 self.reverse_members = {} # ChannelMembers referencing this message
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def __str__(self):
 if self.name == None:
@@ -938,8 +939,7 @@ class ChannelType(Type):
 self.base = base
 self.member_name = None
 self.members = members
-for attr in attribute_list:
-self.attributes[attr[0][1:]] = attr[1:]
+self.attributes = fix_attributes(attribute_list)
 
 def __str__(self):
 if self.name == None:
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH 19/33] Add code to handle destination variable

2015-07-01 Thread Frediano Ziglio
Add some classes to be able to store retrieved data from structure
and messages.
The idea is to generate code dynamically when variable are readed.

Signed-off-by: Frediano Ziglio fzig...@redhat.com
---
 python_modules/dissector.py | 101 
 1 file changed, 101 insertions(+)

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 598cc3b..bd68262b 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -54,6 +54,105 @@ def write_parser_helpers(writer):
 writer.writeln('#endif')
 writer.newline()
 
+# generate code to declare a variable only when needed
+# code is generated only when we read the reference
+class Reference:
+def __init__(self, writer, name):
+self.defined = False
+self.written = False
+self.name = name
+# create a subwriter to write code to read variable only once
+self.writer = writer.get_subwriter()
+
+def write(self, size, value, scope):
+if not size in (8, 16, 32, 64):
+raise Exception('Unknown size %d for %s' % (size, self.name))
+assert(not self.defined or (value, size) == (self.value, self.size))
+if not self.defined:
+self.value = value
+self.size = size
+self.scope = scope
+self.defined = True
+
+def read(self):
+# variable not yet defined
+assert(self.defined)
+if not self.written:
+assert(not self.scope.variable_defined(self.name))
+t = { 8: 'guint32', 16: 'guint32', 32: 'guint32', 64: 'guint64' 
}[self.size]
+self.scope.variable_def(t, self.name)
+self.writer.assign(self.name, self.value)
+self.written = True
+return self.name
+
+class Level:
+def __init__(self, n=0):
+self.level = n
+def __add__(self, n):
+return Level(self.level + n)
+def __sub__(self, n):
+assert(self.level = n)
+return Level(self.level - n)
+def __trunc__(self):
+return self.level
+
+# represent part of a destination to write to
+# for instance if we are parsing a structure dest represent that structure 
output
+class Destination:
+def __init__(self, scope):
+self.refs = {}
+self.is_helper = False
+self.reuse_scope = scope
+self.parent_dest = None
+self.level = Level()
+
+def child_sub(self, member, scope):
+return SubDestination(self, member, scope)
+
+def declare(self, writer):
+return writer.optional_block(self.reuse_scope)
+
+def is_toplevel(self):
+return self.parent_dest == None and not self.is_helper
+
+def read_ref(self, member):
+return self.get_ref(member).read()
+
+def write_ref(self, writer, size, member, value):
+ref = self.get_ref(member, writer)
+ref.write(size, value, self.reuse_scope)
+
+def ref_size(self, member):
+return self.get_ref(member).size
+
+class RootDestination(Destination):
+def __init__(self, scope):
+Destination.__init__(self, scope)
+self.base_var = fld
+
+def get_ref(self, member, writer=None):
+name = (self.base_var + . + member).replace('.', '__')
+if name in self.refs:
+return self.refs[name]
+if not writer:
+raise Exception('trying to read a reference to %s' % member)
+self.refs[name] = ref = Reference(writer, name)
+return ref
+
+def declare(self, writer):
+return writer.no_block(self.reuse_scope)
+
+class SubDestination(Destination):
+def __init__(self, parent_dest, member, scope):
+Destination.__init__(self, scope)
+self.parent_dest = parent_dest
+self.member = member
+self.level = parent_dest.level
+
+def get_ref(self, member, writer=None):
+return self.parent_dest.get_ref(self.member + . + member, writer)
+
+
 def write_msg_parser(writer, message, server):
 msg_name = message.c_name()
 function_name = dissect_spice_%s_%s % ('server' if server else 'client', 
msg_name)
@@ -70,6 +169,8 @@ def write_msg_parser(writer, message, server):
guint32,
GlobalInfo *glb _U_, proto_tree *tree0 
_U_, guint32 offset, True)
 
+dest = RootDestination(parent_scope)
+
 writer.statement(return offset)
 writer.end_block()
 
-- 
2.1.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH] Make monitors config debug output more clear

2015-07-01 Thread Christophe Fergeau
On Tue, Jun 30, 2015 at 12:26:38PM -0500, Jonathon Jongsma wrote:
 Indicate whether the monitors config debug output is from sending or
 receiving new monitors configuration. You can tell this by looking at
 which channel is involved (main vs display), but making it more explicit
 is helpful for glancing through logs.

ACK.

Christophe


pgpkRg2rM1Ac9.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel