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?
>
> *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.
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/42a02a93/attachment.html>
