On Wed, Feb 27, 2013 at 06:24:45PM -0500, Stefan Berger wrote: > On 02/27/2013 05:57 PM, mdroth wrote: > >On Tue, Feb 26, 2013 at 05:03:56PM -0600, jsch...@linux.vnet.ibm.com wrote: > >>These patches implement asn1 ber visitors for encoding and decoding data. > >>References: <20130226230354.982917...@linux.vnet.ibm.com> > >>Content-Disposition: inline; filename=asn1_all.diff > >> > >>Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com> > >>Signed-off-by: Joel Schopp <jsch...@linux.vnet.ibm.com> > >>--- > >> Makefile.objs | 10 > >> ber/Makefile.objs | 2 > >> ber/ber-common.c | 56 ++ > >> ber/ber-input-visitor.c | 969 > >> ++++++++++++++++++++++++++++++++++++++++++++ > >> ber/ber-input-visitor.h | 30 + > >> ber/ber-output-visitor.c | 563 +++++++++++++++++++++++++ > >> ber/ber-output-visitor.h | 28 + > >Please break the input/output implementations out into separate patches, > >and follow those up with test cases. See: > > > >tests/test-{string,qmp}-{input,output}-visitor.c > >tests/test-visitor-serialization.c > > I saw this with the other test cases... The problem with the ASN.1 > encoding is that writing ASN.1 into a buffer doesn't make much > sense, unless of course you were to compare it against a binary > string that was generated through other means, which I didn't do. So > instead I wrote test cases that write the ASN.1 stream using the > output visitor and then immediately feed the streams into the input > visitor and then I compare the outcome against expected outcome, > e.g., that a stucture's fields were properly filled. I hope this > makes more sense in the ASN.1 case.
This is actually exactly what test-visitor-serialization.c does. It feeds various normal/corner cases of visit_type_* into your output visitor, takes that output and feeds it back into the input visitor, then validates it against the original input. Just need to create a SerializeOps implementation to drive your visitor and plug it in like the others. Just doing that would be a good start at least, but it would be really nice to validate the encoding against some other reference implementation. Have you looked into libsnacc? It seems to be the most readilly available library. I wouldn't make it a formal build requirement, but it would be nice to be able to execute test cases that use it if it's present. Even failing that, I personally wouldn't even mind just generating an encoded blob outside the tree, and then checking that it along with the textual description and steps to generate it, then validating the visitors against it. It's not too far off from our "hand-written" JSON to test the QMP visitors. I don't think we have any reasonable assurance that our implementation is correct otherwise. > > Stefan > >