Hi Ginnie,
please see my comments below (only those
not already covered by Dave are mentioned).
Thank you,
Jan
td_mg.c
-------
Since fstyp_ident() allows checking for particular filesystem,
you might avoid using strcmp() and simplifying the code
as well as making life for fstyp_ident() easier, since it wouldn't
need to enumerate trough all known filesystem modules - e.g.
1232 if ((status = fstyp_ident(fstyp_handle, NULL, &fstype)) == 0) {
1233 td_debug_print(LS_DBGLVL_INFO, \
1234 "td_fstyp_check():fstype is %s\n", fstype);
1235 if ((status = strcmp(fstype, fs)) == 0) {
1236 fstyp_fini(fstyp_handle);
1237 (void) close(fd);
1238 return (status);
1239 }
1240 }
->
if ((status = fstyp_ident(fstyp_handle, fs, &fstype)) == 0) {
/* filesystem matches */
} else {
/*
* filesystem is not that type, capture output of
* fstyp_strerror(fstyp_handle, status)
*/
}
Speaking about testing, I can see from bug report that error messages
disappeared after fix was applied - this covers 'ZFS scenario'.
With respect to 'UFS scenario' (the code path where there is UFS filesystem
present on slice), if it was not already covered, I might recommend to run the
installer on system with Solaris instance on UFS and check that the code
path is activated (td_fstyp_check() correctly recognizes UFS filesystem)
and installer reports Solaris instance on it.
Virginia Wray wrote:
> Hi -
>
> Could I get a code review for the following:
> http://defect.opensolaris.org/bz/show_bug.cgi?id=4279
>
> Webrev:
> http://cr.opensolaris.org/~ginnie/4279/
>
> thx,
>
