Hi John.
Thanks for reviewing.
On 09/15/11 05:35 AM, John Fischer wrote:
Jack,
Looks good. Please remove these contractions within the comments:
421 # Explicitly test for a None value as etree accepts it and we
don't.
580 # Explicitly test for a None value as etree accepts it and we
don't.
657 # No path to parent (z doesn't exist). Try to build one
679 # of elements that leads up to it) if it doesn't.
All adjusted.
No further review from me is necessary.
What sort of testing was performed for this fix?
I tested the find_parent_path() in process_dtd from within the python
interpreter, using the ai.dtd, giving various elements, including the
root element, a zvol (which is pretty far away from the root), a bogus
element. I instrumented the routine to make sure it was doing what it
was supposed to.
I tested the changes to add() in mim.py mostly via commandline /
aimanifest(1M). Using AI's default.xml as a starting point, I did
"aimanifest add" of a "disk_name" element with and without its parent
"disk" being previously present in the manifest. I deleted the "target"
clause and added "disk_name@name", then "disk/disk_name@name" and then
"target/disk/disk_name@name" to be sure it handled ancestor elements
correctly. I also verified that additions of same-tagged siblings still
worked by adding two disks as "aimanifest add disk disk1" and "... disk2".
I tested the changes to args checking by doing this:
aimanifest set "" junk
aimanifest set target ""
aimanifest get ""
aimanifest add "" junk
aimanifest add target ""
plus verification of non-empty-string args had already been checked
through other testing done.
Thanks,
Jack
Thanks,
John
On 09/14/11 07:59 PM, Jack Schwartz wrote:
Hi everyone.
Seeking two reviews by Thursday COB and Dave's approval please. I'd
like to request Ethan be one of the reviewers.
Here is a code review that fixes a hole in aimanifest path
processing. Without this bugfix,
aimanifest add diskname@name c0t0d0s0
or other references to non-absolute paths which have no values in
them fails.
webrev:
https://cr.opensolaris.org/action/browse/caiman/schwartz/7090604_1
bug report:
http://monaco.us.oracle.com/detail.jsf?cr=7090604
While it introduces a new method to find the path leading to the
referenced element, all code introduced by this fix affects only the
failing cases; codepaths of working cases are not affected.*
* While in there, I added some additional args checking for set(),
get() and add(), which I have tested but can remove if it makes the
difference between being allowed in or not.
Thanks for your time,
Jack
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
