Bug 38285 - reinstate recursive marshalling test, and other minor stuff from #33870
Summary: reinstate recursive marshalling test, and other minor stuff from #33870
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.5
Hardware: Other All
: low minor
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL: http://cgit.freedesktop.org/~smcv/dbu...
Whiteboard:
Keywords: patch
Depends on:
Blocks: 33870
  Show dependency treegraph
 
Reported: 2011-06-14 02:31 UTC by Simon McVittie
Modified: 2012-02-08 11:13 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH 1/4] dbus-test: only test for memleaks if we actually ran the test (1.04 KB, patch)
2011-06-14 02:34 UTC, Simon McVittie
Details | Splinter Review
[PATCH 2/4] Reinstate the recursive marshalling test but skip the later parts by default (2.86 KB, patch)
2011-06-14 02:34 UTC, Simon McVittie
Details | Splinter Review
[PATCH 3/4] find_field_for_modification: document where the readers are left pointing (1.28 KB, patch)
2011-06-14 02:34 UTC, Simon McVittie
Details | Splinter Review
[PATCH 4/4] dbus-marshal-recursive: make some internal functions static (5.85 KB, patch)
2011-06-14 02:35 UTC, Simon McVittie
Details | Splinter Review
[2/4 v2] Reinstate the recursive marshalling test but skip the later parts by default (2.87 KB, patch)
2011-09-19 07:03 UTC, Simon McVittie
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2011-06-14 02:31:16 UTC
+++ This bug was initially created as a clone of Bug #33870 +++

While fixing Bug #33870 I noticed that:

- if you only run one of the dbus-test sub-tests by putting its name on the
  command line, the skipped tests still produce output when they check for
  memory leaks

- the recursive marshalling test is #if 0, but at least the quicker early
  stages of it should be run

- the arguments to find_field_for_modification are rather opaque

- some of dbus-marshal-recursive's extern API is obscure, and only used
  to realign messages; making it static clarifies its scope
Comment 1 Simon McVittie 2011-06-14 02:34:11 UTC
Created attachment 47931 [details] [review]
[PATCH 1/4] dbus-test: only test for memleaks if we actually ran the  test

This reduces output spam if you're only running one part.
Comment 2 Simon McVittie 2011-06-14 02:34:31 UTC
Created attachment 47932 [details] [review]
[PATCH 2/4] Reinstate the recursive marshalling test but skip the  later parts by default
Comment 3 Simon McVittie 2011-06-14 02:34:51 UTC
Created attachment 47933 [details] [review]
[PATCH 3/4] find_field_for_modification: document where the readers  are left pointing
Comment 4 Simon McVittie 2011-06-14 02:35:14 UTC
Created attachment 47934 [details] [review]
[PATCH 4/4] dbus-marshal-recursive: make some internal functions  static

Having a disabled type writer, comparing type readers by position, and
performing a single step of copying from a reader to a writer all seem pretty
obscure, and were only used within dbus-marshal-recursive.c (to realign
after insertion).
Comment 5 Simon McVittie 2011-09-19 07:03:47 UTC
Created attachment 51333 [details] [review]
[2/4 v2] Reinstate the recursive marshalling test but skip the later parts by default

Updated version of Attachment #47932 [details], which conflicted with more recent changes to master.
Comment 6 Cosimo Alfarano 2012-01-10 09:04:48 UTC
Review++, having no hat though
Comment 7 Simon McVittie 2012-02-08 11:13:41 UTC
(In reply to comment #6)
> Review++, having no hat though

Thanks, fixed in git for 1.5.10.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.