On 11/25/09 01:51 PM, William Schumann wrote:
> Sanjay,
>
> Sanjay Nadkarni wrote:
>> ...
>>>>
>>>> auto_parse.c:
>>>> 193 and others:
>>>> if p is NULL, wouldn't this be an error. If so it will not be
>>>> caught. Similar issues with other parsed values.
>>> Errors could occur in auto_parse_manifest.c during tag parsing that
>>> would be ignored. However, these errors would be very unlikely,
>>> since they would involve a basic breakdown in the embedded Python
>>> interpreter, which has already been invoked to validate the
>>> manifest. Since fixing this would involve many changes in three
>>> different modules, and since this was not introduced by iSCSI, I
>>> would prefer to file a bug on this and handle it later at a more
>>> appropriate priority.
>>
>> Sorry for not being clear. There are 2 issues here:
>>
>> 1. If you truly believe that p != NULL then why not change them to an
>> assert.
> p == NULL when there is no value for the tag in the manifest (as well
> as the Python interpreter errors describe just above). Remember that
> assert() only has an effect in a debug build. The Python errors in
> auto_parse_manifest.c could have assert()s so that these failures
> would cause assertion failures in a debug build. Would that suffice?
Yup that would ok.
>> 2. Sorry for not being clear. I was really asking where is the
>> sematic parsing occurring ?
> The only semantic parsing with these new values is in
> td_iscsi.c:parse_lun_num(), and even that doesn't report parsing
> problems explicitly. Parameters that could benefit from some semantic
> validation are: IP address, LUN. port could be checked for absurd
> values.
Sure..but I what I was getting at was dependency between the various
elements that are defined in the .rng file. They are marked as optional
are they really optional ? The xor test indicates that there is a
dependency/requirement or am I mistaken ?
>>
>>>>
>>>>
>>>> auto_td.c:
>>>> 482: Just to be sure..when "if" statement fails,
>>>> diskiscsiname and diskiscsiip are both either
>>>> defined or undefined. A comment to that effect would be useful.
>>>> 492: I would have thought that rootpath[MAXNAMELEN] would
>>>> be sufficient (i.e. 256) instead of MAXPATHLEN
>>>> which is 1024.
>>> According to RFC 3720, the iSCSI name itself can be up to 223
>>> bytes. Add to that the maximum length of the IP address, port
>>> number, protocol, and LUN, plus the "iscsi:" and separator
>>> characters and this is over 256 bytes.
>> Okay
>>>>
>>>> 539: strncmp(rootpath, "iscsi:", strlen("iscsi:")) would
>>>> be preferable.
>>>>
>> ???
> What's the problem here? The requested change on 539 was made in the
> source and appears in the webrev.
I believe, I email crossed. Ignore.
>
> Thank you,
> William
