Jack Schwartz wrote: > Hi Sundar. > > Thank you for your review. > > On 04/05/09 21:07, Sundar Yamunachari wrote: >> Jack, >> >> *usr/src/cmd/ai-webserver/AI_database.py* >> >> Does lines 531-539 strip leading zeros? For example if the IP address >> is 010002021121, what is the value of ret? Will it be 10:2.21.121 or >> 010.002.021.121? I am asking this because this is stored as string, >> will it give any problems when compared with another IP? > The (MAC) value comes into lines 531-535 as a zero-padded string. > Processing here just adds the colons back. The string is zero-padded > before being stored in the database, as part of verifyXML.py / > __checkMAC (line 156 specifically). > > IP values end up being stored as longs in the database, and I found > out about this in a way similar to what you are asking about. > Initially I used str to convert it, which didn't left-pad with zeros, > and it threw the dots off. This is why on line 537 I convert the > string using %12.12d, which zero-pads on the left. > > Note that this function is used only for output. In order to minimize > risk, I don't change how the data is stored in the database (i.e. > string vs number), and I don't change how database data is compared. > > Does this answer your question? Yes. I wanted to make sure that we don't reject MAC addresses and IP addresses with out padded zeros. >> >> *usr/src/cmd/ai-webserver/verifyXML.py* >> >> 120: The check includes 0 and 255, where as the comment on line 96 >> excludes 0 and 255. Please check and fix it. > Thanks. The code is right. I have updated the comments. > >> 154: Same comment as above. Check the comment in line 130 > Comments updated. >> 220; Will it be possible to have "num_values for range" other than 2? >> Do we need to check num_values != 2 here? > Originally I had this check in place, then I took it out since the > schema only supports ranges of two values, and a range with other than > two values doesn't make sense. The code would be dead code. I'll add > a comment about this to clarify in the code. Okay.
- Sundar > > I'll repost the code review after checking Clay's latest comments. > > Thanks again for reviewing. > > Thanks, > Jack >> >> - Sundar >> >> Jack Schwartz wrote: >>> Hi everyone. >>> >>> Here is a webrev of my proposed fixes for: >>> 4325 Better syntactic treatment of IP and MAC address AI criteria >>> >>> A medium-level description of the changes is in the bug-report: >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4325 >>> >>> Webrev: >>> http://cr.opensolaris.org/~schwartz/090402.1/webrev/ >>> >>> Testing done: >>> >>> Ran publish manifest and verified with webserver: >>> - MAC: min/unbounded, IP: unbounded/max, network: min/unbounded, >>> mem: single <value> for range, minMAC has single digit btwn >>> colons >>> - range with three values: correctly failed validation >>> - range with two unbounded values: correctly failed validation >>> - range with 0 minimum: correctly passed validation >>> - single IP addr given as a <range>: corectly failed validation >>> - pair of values given as a <value>: correctly failed validation >>> - 2 values for non range criterion, surrounded by <value>: correctly >>> failed validation >>> - single value for non range criterion, surrounded by <value>: >>> passed validation >>> - 2 values for non range criterion, surrounded by <range>: correctly >>> failed validation >>> - Try a combination with a non-numeric value (arch=sparc): worked >>> Also verified list-manifests displayed data correctly. >>> >>> I'm requesting that Clay review, as he's most familiar with the code >>> I've changed and he has a heads-up that it's coming. Others are >>> invited to review as well. >>> >>> Tomorrow I'll verify that an install starts as it should, but I have >>> confidence it will work as the webserver output looks fine. >>> >>> Sorry to rush. I'm requesting a review ASAP, by COB tomorrow if >>> possible. I'm supposed to be done on Friday, but to give less than >>> 1 day for this review is not really realistic. >>> >>> Thanks, >>> Jack >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20090406/382842c0/attachment.html>
