Thanks for the explanations William.
William Schumann wrote: > Joe, > > Joseph J VLcek wrote: >> William Schumann wrote: >>> New Automated Installer code allows creating partitions of different >>> types. >>> >>> Place partition type value in element "partition_type". >>> >>> Allows "FAT32", "DOS16", "SOLARIS", "OPENSOLARIS" or any numeric type >>> specified in fdisk.h >>> >>> http://cr.opensolaris.org/~wmsch/bug-6584/ >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=6584 >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >> Looks good William. >> >> Please confirm my question, which is a possible confusion on my part >> My suggestion may just be a style issue but it seemed clearer to me... >> >> Hope this helps! >> >> Joe >> >> Question: >> --------- >> File: usr/src/cmd/auto-install/ai_manifest.rng >> >> Please confirm line 195 correct. >> >> 195 <text/> > Yes, this is the way that a text field is defined in an RNG file. The / > at the end means that there is no closing </text> tag in XML. >> >> Just a suggestion: >> ------------------ >> File: usr/src/cmd/auto-install/auto_parse.c >> >> Would it seem clearer to negate the logic on line 362 and remove the >> continue on line 362? >> >> From: >> >> 362 if (errno == 0 && endptr != p[i]) >> 363 continue; >> 364 auto_debug_print(AUTO_DBGLVL_ERR, >> 365 "Partition type in manifest >> (%s) is " >> 366 "not a valid number or disk >> format.\n", >> 367 p[i]); >> 368 *pstatus = 1; >> 369 free(api); >> 370 errno = 0; >> 371 return (NULL); >> >> >> To: >> >> if (!(errno == 0 && endptr != p[i])) { >> auto_debug_print(AUTO_DBGLVL_ERR, >> "Partition type in manifest (%s) is " >> "not a valid number or disk format.\n", >> p[i]); >> *pstatus = 1; >> free(api); >> errno = 0; >> return (NULL); >> } > BTW, I changed the wording when I revised the webrev in response to > Jan's comments. > > If I were to negate the conditional expression, I would use !(A && B) = > (!A || !B), or (errno != 0 || endptr == p[i]). > > At this point in the code, we are at the 4th indentation level, so the > maximum line length in Solaris coding standard of 80 characters will > result in very short lines, which are increasingly messy and harder to > read. I use the continue; to avoid yet another level of indentation, > allowing the lines of the logging statement to be slightly longer. > Otherwise the logging statement would look like this. >> auto_debug_print(AUTO_DBGLVL_ERR, >> "Partition type in manifest (%s) " >> "is not a valid number or " >> "partition type.\n", > This use of continue; to skip the error handler might not be a very > common practice, but it should still be easily understandable. > > William
