Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
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.
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.
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.
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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