exporting dbus-marshal-recursive.h

[Havoc Pennington]

Hi,

The tests for this signature stuff should conceivably be in dbus-
marshal-recursive-util.c, so you could use value_generator() to get a
bunch of different signatures to test each thing against. Not a critical
thing to do right away though.

On Sat, 2005-02-19 at 12:58 -0500, Colin Walters wrote:
> Yeah, the problem is that I need to store two large-ish structures both
> of which could potentially grow substantially.  I made an initial guess
> about how large DBusStructureRealIter should be (including future
> padding), and it wasn't even large enough to hold the existing
> DBusString and DBusTypeReader.

I think we're large enough here to be worth fixing in some way. This
iterator only needs to be a single pointer or so in theory.

Maybe the right approach is to just reimplement. The only remotely
complicated part is skip_one_complete_type() which can be factored out
of dbus-marshal-recursive.c. Also, you can trivially get great test
coverage by comparing the results of the new code with dbus-marshal-
recursive

I think the implementation is this short:

struct
{
 char *p;
 unsigned int finished : 1;
 unsigned int is_array : 1;
} SigReader;

init (reader, s)
{
  reader->p = s;
  reader->finished = FALSE;
  reader->is_array = FALSE; /* can never be inside array while at
toplevel */
}

next (reader)
{
  if (reader->finished)
    return FALSE;
  else
{
  if (reader->is_array)
    {
     reader->finished = TRUE;
     return FALSE;
    }
  skip_one_complete_type_c_str (&reader->p); /* factored out from
current code */
  if (*reader->p == END_STRUCT_CHAR || *reader->p ==
END_DICT_ENTRY_CHAR)
   {
    reader->finished = TRUE;
    return FALSE;
   }
  return *reader->p != DBUS_TYPE_INVALID;
  }
}

get_current_type (reader)
{
  if (reader->finished)
    return DBUS_TYPE_INVALID;
  else
    return _dbus_first_type_in_signature_c_str (reader->p);
}

recurse (reader, sub)
{
  *sub = *reader;
   if (get_current_type (reader) == array)
    sub->is_array = TRUE;
} 

Maybe I'm missing a complexity.

If we do this it's possible we should rip the types only reader stuff
out of dbus-marshal-recursive.[hc] which could both save bloat and speed
up message parsing.

> Ok, I fixed this.

Shouldn't check_memleaks() be in run_test in addition to run_data_test
()? Or were there existing tests where memleaks weren't checked?

> In addition, this patch:
> 
> o Implements the human-readable format
>   - Added to documentation
>   - New methods dbus_signature_to_human_readable,
>     dbus_signature_from_human_readable

Suggestion here is to avoid the term "pretty" because it implies a
nonrigorous or modifiable-at-whim format, while this is intended to be a
machine-readable format that is also human readable. I guess "human
readable" isn't a great name either for this reason.

Maybe we should just call it "long" or "expanded" format?
dbus_signature_expand(), dbus_signature_unexpand() or something.

Also, maybe signatures in this format should start with a special
character so that functions can allow either format. Maybe ':' since we
already use that to mark special bus names. Then the introspection
format can allow either one. Kind of bizarre though so if there are no
other use cases beyond introspection we should probably skip it or use
two different attribute names (type=sig and type_long=expanded or
something)

> o Added method dbus_signature_typecode_is_basic, to know when to recurse

I would just move/rename _dbus_type_is_basic() to dbus_type_is_basic()
probably, along with the related functions. Maybe type_to_string (rename
to "type to long"?), type_is_container, type_is_fixed_length.

> o Added dbus_signature_iter_init_validate which validates signature

To me just having dbus_signature_validate() (which should probably
return a DBusError) separate from _init() would be nicer.

> o Added documentation for new signature bits
> o Tests all the above

Thanks for those two!

Havoc