[Spice-devel,v3,38/51] Use a class to register wireshark fields

Submitted by Frediano Ziglio on July 21, 2015, 4:46 p.m.

Details

Message ID 1437497181-26929-39-git-send-email-fziglio@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Frediano Ziglio July 21, 2015, 4:46 p.m.
Allow to reuse code and check better if field was already registered

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 python_modules/dissector.py | 58 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 5639baa..c88118f 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -21,8 +21,49 @@  def new_ett(writer):
     return name
 
 
+# handle registration of wireshark flags
 hf_writer = None
 hf_defs = None
+class HF:
+    fields = {}
+
+    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)
+
+    # 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):
+        other = self.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]:
+                    raise Exception('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':
+                    raise Exception('HF Field different from previous for\n\t%s\n\t%s' % (other, self))
+            return
+
+        self.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('},')
 
 
 # handle wireshark attributes
@@ -306,16 +347,13 @@  def write_wireshark_field(writer, container, member, t, ws, tree, size, encoding
     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()
 
 
 # Note: during parsing, byte_size types have been converted to count during validation

Comments

On Tue, Jul 21, 2015 at 05:46:08PM +0100, Frediano Ziglio wrote:
> Allow to reuse code and check better if field was already registered
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  python_modules/dissector.py | 58 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/python_modules/dissector.py b/python_modules/dissector.py
> index 5639baa..c88118f 100644
> --- a/python_modules/dissector.py
> +++ b/python_modules/dissector.py
> @@ -21,8 +21,49 @@ def new_ett(writer):
>      return name
>  
>  
> +# handle registration of wireshark flags
>  hf_writer = None
>  hf_defs = None
> +class HF:

Just curious about the naming here, is 'HF' something which is used in
wireshark? I'd prefer a longer, more expressive name for a class name

> +    fields = {}
> +
> +    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)
> +
> +    # 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):
> +        other = self.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]:
> +                    raise Exception('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':
> +                    raise Exception('HF Field different from previous for\n\t%s\n\t%s' % (other, self))
> +            return
> +
> +        self.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('},')
>  
>  
>  # handle wireshark attributes
> @@ -306,16 +347,13 @@ def write_wireshark_field(writer, container, member, t, ws, tree, size, encoding
>      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()
>  
>  
>  # 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
> 
> On Tue, Jul 21, 2015 at 05:46:08PM +0100, Frediano Ziglio wrote:
> > Allow to reuse code and check better if field was already registered
> > 
> > Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> > ---
> >  python_modules/dissector.py | 58
> >  +++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 48 insertions(+), 10 deletions(-)
> > 
> > diff --git a/python_modules/dissector.py b/python_modules/dissector.py
> > index 5639baa..c88118f 100644
> > --- a/python_modules/dissector.py
> > +++ b/python_modules/dissector.py
> > @@ -21,8 +21,49 @@ def new_ett(writer):
> >      return name
> >  
> >  
> > +# handle registration of wireshark flags
> >  hf_writer = None
> >  hf_defs = None
> > +class HF:
> 
> Just curious about the naming here, is 'HF' something which is used in
> wireshark? I'd prefer a longer, more expressive name for a class name
> 

Yes, wireshark thing. All fields have this hf prefix. The "f" surely stand for field... about "h" I don't exactly know.

Frediano

> > +    fields = {}
> > +
> > +    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)
> > +
> > +    # 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):
> > +        other = self.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]:
> > +                    raise Exception('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':
> > +                    raise Exception('HF Field different from previous
> > for\n\t%s\n\t%s' % (other, self))
> > +            return
> > +
> > +        self.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('},')
> >  
> >  
> >  # handle wireshark attributes
> > @@ -306,16 +347,13 @@ def write_wireshark_field(writer, container, member,
> > t, ws, tree, size, encoding
> >      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()
> >  
> >  
> >  # 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
>