Roland, on ksh, Roland Mainz wrote: > William Schumann wrote: > >> http://cr.opensolaris.org/~wmsch/bug-1559/ >> http://defect.opensolaris.org/bz/show_bug.cgi?id=1559 >> > [snip] > > Quick look at http://cr.opensolaris.org/~wmsch/bug-1559/mkmenu.patch > (patch code is quoted with "> "): > >> + #presence of /.liveusb indicates install from USB disk >> + #for USB installation the GRUB map will list the USB device >> + # as the first disk, which is very likely not the normal order >> + # reset the normal order by decrementing the disk index >> + if [ -e "/.liveusb" ] && [ $gdisk -gt 0 ]; then >> > > Please use [[ ]] when operating on paths and use arithemtric expressions > to compare integer variables. > The OpenSolaris ksh man page supports your claims. -gt is listed as being supported, but obsolete. > The line would then look like this: > -- snip -- > if [[ -e "/.liveusb" ]] && (( gdisk > 0 )); then > -- snip -- > > >> + #identify windows partitions and decrement >> + #disk index for USB installations >> + case $id in >> + 7|11|12|18|23|222) >> + root="(hd$(($gdisk-1)),$gpart)" >> > > 1. Please use ${varname} instead of $varname to make this code more > readable. > I will just defer to you on this one, although it is not the convention used elsewhere in this script and although I do not find it more readable. > 2. The '$' character to expand a variable's content within arithemtric > expression is not needed and may even be harmfull (see > http://www.opensolaris.org/os/project/shell/shellstyle/#avoid_unneccesary_string_number_conversions) > Changed gpart=$partition-1 to (( gpart=partition-1 )) as well. > AFAIK the line should look like this: > -- snip -- > root="(hd$((gdisk-1)),${gpart})" > -- snip -- > > >> + ;; >> + *) root="(hd$gdisk,$gpart)" ;; >> > > Same as above, the line should look like this: > -- snip -- > root="(hd${gdisk},${gpart})" ;; > -- snip -- > > >> + esac >> + else >> + root="(hd$gdisk,$gpart)" >> > > Same as above, the line should be: > -- snip -- > root="(hd${gdisk},${gpart})" > -- snip -- > > So, I took your advice on all indicated points.
I've not been able to create any test case which breaks the original code changes, but I am pleased to have the code follow safe conventions. Thank you, William >> + fi >> > > ---- > > Bye, > Roland > >
