On Tue, Jul 26, 2016 at 08:16:59AM +0200, Maxime Ripard wrote: > On Tue, Jul 26, 2016 at 10:18:59AM +1000, David Gibson wrote: > > > > Again, off-by-one in this test, I think. Since there are so many > > > > tricky edge cases here, it might be worth making testcases for them. > > > > > > What do you want here? That we move the parsing code out of that loop, > > > make it public and put the prototype in libfdt_internal, or that we > > > craft some DT that would outline all the possible issues with that > > > function, and just test the return code of fdt_overlay_apply? > > > > I was thinking crafted DTs. But I've realized that doing usefully is > > pretty tricky, since overruns probably won't cause an immediate > > problem most of the time. I guess they'd trip errors under valgrind > > at least (as long you make sure there's at least one byte's alignment > > gap until the next tag). > > Is that a general comment? > > I'm still not quite sure what you expect from me. Do you want to test > just that function, or only the fixup parsing function? Should I run > valgrind, or are you expecting it to be done later? Sorry, I wasn't very clear. You don't necessarily need to do anything. But the fact that we've been through several iterations with problems in the edge cases of parsing here suggests that this is tricky and fragile, so tests would be a good idea if possible. Because these are for cases with bad input, you wouldn't need to check for any particular results - just that the functions don't crash or otherwise do bad things when given the bad input. It's probably easier to construct tests which will have bad behaviour that valgrind can detect than ones which have bad behaviour which is more obvious. The whole testsuite can be run under valgrind with "make checkm". It's a good idea to run that every so often - I don't do it routinely because of course it's much, much slower than running the testsuite normally. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
