Hi Sue,
Susan Sohn wrote:
> Hi Jan,
>
> Thank you very much for your code review comments. See my responses
> below to selected comments. The rest of the comments will be addressed
> by Sundar.
>
> >
> >
> > cmd/installadm/create-client.sh
> > -------------------------------
> > 241,269 - it seems these checks are redundant, they were
> > already done on lines 160,192 particularly
>
> At 160, we are just verifying that there *is* an IMAGE_PATH value.
> At 192, I am not seeing anything about IMAGE_PATH?
> For 241 and 269, if you mean 227 and 240, the first verifies that the
> path actually exists, the second verifies that it is a valid boot
> image. Is that what you're looking at and does that answer your question?
My apologies - I was probably looking at the old webrev.
Looking at the latest one, I meant to refer to the following code:
195 if [ -z "${MAC_ADDR}" -o -z "${SERVICE_NAME}" ]; then
196 echo "${myname}: Missing one or more required options."
197 usage
198 fi
[...]
256 if [ "X${MAC_ADDR}" != "X" ] ; then
Since MAC_ADDR is mandatory, it can't be empty -
I think check on line 256 is redundant.
As far as code related to 'IMAGE_PATH' is concerned,
I agree with you - since it is not mandatory, it would
be empty when not provided by the user and the check on
line 227 is required.
Thank you,
Jan
