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?
> 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.
>
>>>
>>>
>>> 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.
Thank you,
William
