1. 07 6月, 2016 1 次提交
  2. 12 5月, 2016 3 次提交
    • E
      qapi: Simplify semantics of visit_next_list() · d9f62dde
      Eric Blake 提交于
      The semantics of the list visit are somewhat baroque, with the
      following pseudocode when FooList is used:
      
      start()
      for (prev = head; cur = next(prev); prev = &cur) {
          visit(&cur->value)
      }
      
      Note that these semantics (advance before visit) requires that
      the first call to next() return the list head, while all other
      calls return the next element of the list; that is, every visitor
      implementation is required to track extra state to decide whether
      to return the input as-is, or to advance.  It also requires an
      argument of 'GenericList **' to next(), solely because the first
      iteration might need to modify the caller's GenericList head, so
      that all other calls have to do a layer of dereferencing.
      
      Thankfully, we only have two uses of list visits in the entire
      code base: one in spapr_drc (which completely avoids
      visit_next_list(), feeding in integers from a different source
      than uint8List), and one in qapi-visit.py.  That is, all other
      list visitors are generated in qapi-visit.c, and share the same
      paradigm based on a qapi FooList type, so we can refactor how
      lists are laid out with minimal churn among clients.
      
      We can greatly simplify things by hoisting the special case
      into the start() routine, and flipping the order in the loop
      to visit before advance:
      
      start(head)
      for (tail = *head; tail; tail = next(tail)) {
          visit(&tail->value)
      }
      
      With the simpler semantics, visitors have less state to track,
      the argument to next() is reduced to 'GenericList *', and it
      also becomes obvious whether an input visitor is allocating a
      FooList during visit_start_list() (rather than the old way of
      not knowing if an allocation happened until the first
      visit_next_list()).  As a minor drawback, we now allocate in
      two functions instead of one, and have to pass the size to
      both functions (unless we were to tweak the input visitors to
      cache the size to start_list for reuse during next_list, but
      that defeats the goal of less visitor state).
      
      The signature of visit_start_list() is chosen to match
      visit_start_struct(), with the new parameters after 'name'.
      
      The spapr_drc case is a virtual visit, done by passing NULL for
      list, similarly to how NULL is passed to visit_start_struct()
      when a qapi type is not used in those visits.  It was easy to
      provide these semantics for qmp-output and dealloc visitors,
      and a bit harder for qmp-input (several prerequisite patches
      refactored things to make this patch straightforward).  But it
      turned out that the string and opts visitors munge enough other
      state during visit_next_list() to make it easier to just
      document and require a GenericList visit for now; an assertion
      will remind us to adjust things if we need the semantics in the
      future.
      
      Several pre-requisite cleanup patches made the reshuffling of
      the various visitors easier; particularly the qmp input visitor.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1461879932-9020-24-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      d9f62dde
    • E
      qapi: Fix string input visitor handling of invalid list · 74f24cb6
      Eric Blake 提交于
      As shown in the previous commit, the string input visitor was
      treating bogus input as an empty list rather than an error.
      Fix parse_str() to set errp, then the callers to exit early if
      an error was reported.
      
      Meanwhile, fix the testsuite to use the generated
      qapi_free_int16List() instead of rolling our own, and to
      validate the fixed behavior, while at the same time documenting
      one more change that we'd like to make in a later patch (a
      failed visit_start_list should guarantee a NULL pointer,
      regardless of what things were on input).
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1461879932-9020-23-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      74f24cb6
    • M
      tests/string-input-visitor: Add negative integer tests · 73374683
      Markus Armbruster 提交于
      Add two negative tests, one for int and one for int16List.  The latter
      exposes a bug: nonsensical input results in an empty list instead of
      an error.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <1461325048-14122-1-git-send-email-armbru@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1461879932-9020-22-git-send-email-eblake@redhat.com>
      73374683
  3. 23 3月, 2016 1 次提交
    • M
      include/qemu/osdep.h: Don't include qapi/error.h · da34e65c
      Markus Armbruster 提交于
      Commit 57cb38b3 included qapi/error.h into qemu/osdep.h to get the
      Error typedef.  Since then, we've moved to include qemu/osdep.h
      everywhere.  Its file comment explains: "To avoid getting into
      possible circular include dependencies, this file should not include
      any other QEMU headers, with the exceptions of config-host.h,
      compiler.h, os-posix.h and os-win32.h, all of which are doing a
      similar job to this file and are under similar constraints."
      qapi/error.h doesn't do a similar job, and it doesn't adhere to
      similar constraints: it includes qapi-types.h.  That's in excess of
      100KiB of crap most .c files don't actually need.
      
      Add the typedef to qemu/typedefs.h, and include that instead of
      qapi/error.h.  Include qapi/error.h in .c files that need it and don't
      get it now.  Include qapi-types.h in qom/object.h for uint16List.
      
      Update scripts/clean-includes accordingly.  Update it further to match
      reality: replace config.h by config-target.h, add sysemu/os-posix.h,
      sysemu/os-win32.h.  Update the list of includes in the qemu/osdep.h
      comment quoted above similarly.
      
      This reduces the number of objects depending on qapi/error.h from "all
      of them" to less than a third.  Unfortunately, the number depending on
      qapi-types.h shrinks only a little.  More work is needed for that one.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      [Fix compilation without the spice devel packages. - Paolo]
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      da34e65c
  4. 16 2月, 2016 1 次提交
  5. 09 2月, 2016 1 次提交
    • E
      qapi: Swap visit_* arguments for consistent 'name' placement · 51e72bc1
      Eric Blake 提交于
      JSON uses "name":value, but many of our visitor interfaces were
      called with visit_type_FOO(v, &value, name, errp).  This can be
      a bit confusing to have to mentally swap the parameter order to
      match JSON order.  It's particularly bad for visit_start_struct(),
      where the 'name' parameter is smack in the middle of the
      otherwise-related group of 'obj, kind, size' parameters! It's
      time to do a global swap of the parameter ordering, so that the
      'name' parameter is always immediately after the Visitor argument.
      
      Additional reason in favor of the swap: the existing include/qjson.h
      prefers listing 'name' first in json_prop_*(), and I have plans to
      unify that file with the qapi visitors; listing 'name' first in
      qapi will minimize churn to the (admittedly few) qjson.h clients.
      
      Later patches will then fix docs, object.h, visitor-impl.h, and
      those clients to match.
      
      Done by first patching scripts/qapi*.py by hand to make generated
      files do what I want, then by running the following Coccinelle
      script to affect the rest of the code base:
       $ spatch --sp-file script `git grep -l '\bvisit_' -- '**/*.[ch]'`
      I then had to apply some touchups (Coccinelle insisted on TAB
      indentation in visitor.h, and botched the signature of
      visit_type_enum() by rewriting 'const char *const strings[]' to
      the syntactically invalid 'const char*const[] strings').  The
      movement of parameters is sufficient to provoke compiler errors
      if any callers were missed.
      
          // Part 1: Swap declaration order
          @@
          type TV, TErr, TObj, T1, T2;
          identifier OBJ, ARG1, ARG2;
          @@
           void visit_start_struct
          -(TV v, TObj OBJ, T1 ARG1, const char *name, T2 ARG2, TErr errp)
          +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
           { ... }
      
          @@
          type bool, TV, T1;
          identifier ARG1;
          @@
           bool visit_optional
          -(TV v, T1 ARG1, const char *name)
          +(TV v, const char *name, T1 ARG1)
           { ... }
      
          @@
          type TV, TErr, TObj, T1;
          identifier OBJ, ARG1;
          @@
           void visit_get_next_type
          -(TV v, TObj OBJ, T1 ARG1, const char *name, TErr errp)
          +(TV v, const char *name, TObj OBJ, T1 ARG1, TErr errp)
           { ... }
      
          @@
          type TV, TErr, TObj, T1, T2;
          identifier OBJ, ARG1, ARG2;
          @@
           void visit_type_enum
          -(TV v, TObj OBJ, T1 ARG1, T2 ARG2, const char *name, TErr errp)
          +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
           { ... }
      
          @@
          type TV, TErr, TObj;
          identifier OBJ;
          identifier VISIT_TYPE =~ "^visit_type_";
          @@
           void VISIT_TYPE
          -(TV v, TObj OBJ, const char *name, TErr errp)
          +(TV v, const char *name, TObj OBJ, TErr errp)
           { ... }
      
          // Part 2: swap caller order
          @@
          expression V, NAME, OBJ, ARG1, ARG2, ERR;
          identifier VISIT_TYPE =~ "^visit_type_";
          @@
          (
          -visit_start_struct(V, OBJ, ARG1, NAME, ARG2, ERR)
          +visit_start_struct(V, NAME, OBJ, ARG1, ARG2, ERR)
          |
          -visit_optional(V, ARG1, NAME)
          +visit_optional(V, NAME, ARG1)
          |
          -visit_get_next_type(V, OBJ, ARG1, NAME, ERR)
          +visit_get_next_type(V, NAME, OBJ, ARG1, ERR)
          |
          -visit_type_enum(V, OBJ, ARG1, ARG2, NAME, ERR)
          +visit_type_enum(V, NAME, OBJ, ARG1, ARG2, ERR)
          |
          -VISIT_TYPE(V, OBJ, NAME, ERR)
          +VISIT_TYPE(V, NAME, OBJ, ERR)
          )
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-Id: <1454075341-13658-19-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      51e72bc1
  6. 19 6月, 2014 3 次提交
  7. 09 5月, 2014 1 次提交
  8. 18 2月, 2014 1 次提交
  9. 07 2月, 2013 1 次提交
    • P
      tests/test-string-input-visitor: Handle errors provoked by fuzz test · 01845438
      Peter Maydell 提交于
      It's OK and expected for visitors to return errors when presented with
      the fuzz test's random data. Since the fuzzer doesn't care about
      errors, we pass in NULL rather than an Error**. This fixes a bug in
      the fuzzer where it was passing the same Error** into each visitor,
      with the effect that once one visitor returned an error, each later
      visitor would notice that it had been passed in an Error** representing
      an already set error, and do nothing.
      
      For the case of visit_type_str() we also need to handle the case where
      an error means that the visitor doesn't set our char*. We initialize
      the pointer to NULL so we can safely g_free() it regardless of whether
      the visitor allocated a string for us or not.
      
      This fixes a problem where this test failed the MacOSX malloc()
      consistency checks and might segfault on other platforms [due
      to calling free() on an uninitialized pointer variable when
      visit_type_str() failed.].
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      Reviewed-by: NAndreas Färber <afaerber@suse.de>
      Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>
      01845438
  10. 26 1月, 2013 1 次提交
  11. 19 12月, 2012 2 次提交
  12. 30 3月, 2012 1 次提交
  13. 21 2月, 2012 1 次提交