[Spice-devel] [PATCH spice-common 7/9] codegen: Remove support for --ptrsize
Frediano Ziglio
fziglio at redhat.com
Thu Mar 7 21:01:02 UTC 2019
>
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
>
Thanks, looking at reply to 9/9 however I think one thing is not clear from
what I wrote in the commit message.
The reason is not only that was used in protocol 1 only and now is not used
but also that it was wrong from the beginning and useless. I would keep it
if I found it useful.
> On Sun, Mar 03, 2019 at 07:10:28PM +0000, Frediano Ziglio wrote:
> > This option was used in protocol 1 to generate 64 bit pointers.
> > A pointer in the protocol is an offset in the current message.
> > This allows the possibility to have messages with pointers with
> > more than 4GB. This feature was removed and not used in protocol 2.
> > The reason is that messages more than 4GB would cause:
Maybe this could be changed in
"The reason this feature was correctly removed in protocol 2 is that
having 64 bit pointers in the protocol would require messages larger
than 4GB which would cause:"
> > - huge latency as a single message would take more than 4 seconds
> > to be send in a 10Gb connection;
> > - huge memory requirements.
> >
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> > common/marshaller.c | 14 +++-----------
> > common/marshaller.h | 2 +-
> > python_modules/marshal.py | 2 +-
> > python_modules/ptypes.py | 18 +++---------------
> > spice_codegen.py | 4 ----
> > 5 files changed, 8 insertions(+), 32 deletions(-)
> >
> > diff --git a/common/marshaller.c b/common/marshaller.c
> > index 4379aa6..c77129b 100644
> > --- a/common/marshaller.c
> > +++ b/common/marshaller.c
> > @@ -96,7 +96,6 @@ typedef struct SpiceMarshallerData SpiceMarshallerData;
> > typedef struct {
> > SpiceMarshaller *marshaller;
> > int item_nr;
> > - int is_64bit;
> > size_t offset;
> > } MarshallerRef;
> >
> > @@ -419,13 +418,13 @@ SpiceMarshaller
> > *spice_marshaller_get_submarshaller(SpiceMarshaller *m)
> > return m2;
> > }
> >
> > -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m, int is_64bit)
> > +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m)
> > {
> > SpiceMarshaller *m2;
> > uint8_t *p;
> > int size;
> >
> > - size = is_64bit ? 8 : 4;
> > + size = 4;
> >
> > p = spice_marshaller_reserve_space(m, size);
> > memset(p, 0, size);
> > @@ -433,7 +432,6 @@ SpiceMarshaller
> > *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m, int
> > m2->pointer_ref.marshaller = m;
> > m2->pointer_ref.item_nr = m->n_items - 1;
> > m2->pointer_ref.offset = m->items[m->n_items - 1].len - size;
> > - m2->pointer_ref.is_64bit = is_64bit;
> >
> > return m2;
> > }
> > @@ -538,13 +536,7 @@ void spice_marshaller_flush(SpiceMarshaller *m)
> > for (m2 = m; m2 != NULL; m2 = m2->next) {
> > if (m2->pointer_ref.marshaller != NULL && m2->total_size > 0) {
> > ptr_pos = lookup_ref(&m2->pointer_ref);
> > - if (m2->pointer_ref.is_64bit) {
> > - write_uint64(ptr_pos,
> > - spice_marshaller_get_offset(m2));
> > - } else {
> > - write_uint32(ptr_pos,
> > - spice_marshaller_get_offset(m2));
> > - }
> > + write_uint32(ptr_pos, spice_marshaller_get_offset(m2));
> > }
> > }
> > }
> > diff --git a/common/marshaller.h b/common/marshaller.h
> > index 041d16e..a631c85 100644
> > --- a/common/marshaller.h
> > +++ b/common/marshaller.h
> > @@ -53,7 +53,7 @@ size_t spice_marshaller_get_offset(SpiceMarshaller *m);
> > size_t spice_marshaller_get_size(SpiceMarshaller *m);
> > size_t spice_marshaller_get_total_size(SpiceMarshaller *m);
> > SpiceMarshaller *spice_marshaller_get_submarshaller(SpiceMarshaller *m);
> > -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m, int is_64bit);
> > +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m);
> > int spice_marshaller_fill_iovec(SpiceMarshaller *m, struct iovec *vec,
> > int n_vec, size_t skip_bytes);
> > void *spice_marshaller_add_uint64(SpiceMarshaller *m, uint64_t v);
> > diff --git a/python_modules/marshal.py b/python_modules/marshal.py
> > index 4e98993..74f3a54 100644
> > --- a/python_modules/marshal.py
> > +++ b/python_modules/marshal.py
> > @@ -234,7 +234,7 @@ def write_array_marshaller(writer, member, array,
> > container_src, scope):
> > def write_pointer_marshaller(writer, member, src):
> > t = member.member_type
> > ptr_func = write_marshal_ptr_function(writer, t.target_type)
> > - submarshaller = "spice_marshaller_get_ptr_submarshaller(m, %d)" % (1
> > if member.get_fixed_nw_size() == 8 else 0)
> > + submarshaller = "spice_marshaller_get_ptr_submarshaller(m)"
> > if member.has_attr("marshall"):
> > rest_args = ""
> > if t.target_type.is_array():
> > diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
> > index 64c198e..bd5f542 100644
> > --- a/python_modules/ptypes.py
> > +++ b/python_modules/ptypes.py
> > @@ -4,8 +4,6 @@ import types
> > _types_by_name = {}
> > _types = []
> >
> > -default_pointer_size = 4
> > -
> > def type_exists(name):
> > return name in _types_by_name
> >
> > @@ -512,7 +510,6 @@ class PointerType(Type):
> > Type.__init__(self)
> > self.name = None
> > self.target_type = target_type
> > - self.pointer_size = default_pointer_size
> >
> > def __str__(self):
> > return "%s*" % (str(self.target_type))
> > @@ -521,9 +518,6 @@ class PointerType(Type):
> > self.target_type = self.target_type.resolve()
> > return self
> >
> > - def set_ptr_size(self, new_size):
> > - self.pointer_size = new_size
> > -
> > def is_fixed_nw_size(self):
> > return True
> >
> > @@ -531,19 +525,13 @@ class PointerType(Type):
> > return True
> >
> > def primitive_type(self):
> > - if self.pointer_size == 4:
> > - return "uint32"
> > - else:
> > - return "uint64"
> > + return "uint32"
> >
> > def get_fixed_nw_size(self):
> > - return self.pointer_size
> > + return 4
> >
> > def c_type(self):
> > - if self.pointer_size == 4:
> > - return "uint32_t"
> > - else:
> > - return "uint64_t"
> > + return "uint32_t"
> >
> > def contains_extra_size(self):
> > return True
> > diff --git a/spice_codegen.py b/spice_codegen.py
> > index 66f99a5..e518686 100755
> > --- a/spice_codegen.py
> > +++ b/spice_codegen.py
> > @@ -171,8 +171,6 @@ parser.add_option("-i", "--include",
> > help="Include FILE in generated code")
> > parser.add_option("--prefix", dest="prefix",
> > help="set public symbol prefix", default="")
> > -parser.add_option("--ptrsize", dest="ptrsize",
> > - help="set default pointer size", default="4")
> > parser.add_option("--license", dest="license",
> > help="license to use for generated file(s) (LGPL/BSD)",
> > default="LGPL")
> > parser.add_option("--generated-header", dest="generated_header",
> > metavar="FILE",
> > @@ -188,8 +186,6 @@ if len(args) == 0:
> > if len(args) == 1:
> > parser.error("No destination file specified")
> >
> > -ptypes.default_pointer_size = int(options.ptrsize)
> > -
> > proto_file = args[0]
> > dest_file = args[1]
> > proto = spice_parser.parse(proto_file)
More information about the Spice-devel
mailing list