Hey Jack.
I had learned a lot from Roland Mainz when I updated usbgen a while back.
My comments share what I had learned.
Issue 1:
--------
Some ksh93 guidelines I wrote up to capture what I had learned from
Roland can be found at:
www.opensolaris.org/os/project/shell/shellstyle/
I've also attached a copy to this email.
Issue 2:
--------
shcomp can be used to confirm this script compiles
Issue 3:
--------
Before line 123 confirm the file passed to strip_grub_hd_entry exists
and if it does not invoke error_handler.
e.g.:
if [[ ! -f "${1}" ]] ; then
error_handler "Unable to access file ${1}"
fi
Issue 4:
--------
Regarding hdfound defined on line:
122 hdfound="False"
- use true and false ksh93 builtin commands which return
either zero or non-zero and are directly consumed by the
"if... then"-construct
e.g.
Don't do:
---------
var_x="TRUE"
if [[ "${var_x}" == "FALSE" ]]; then
Do:
---
var_x = true
if ${var_x} ; then
Issue 5:
--------
125 echo ${oneline} | grep "Hard Disk" >/dev/null
132 echo ${oneline} | grep "^title" >/dev/null
The use of "grep" (which should really be "fgrep") can be avoided by
using the builtin extended regualr expression pattern matching
facilities.
e.g.
$ oneline="a line with ard Disk in it"
$ [[ "${oneline}" == ~(E).*Hard\ Disk.* ]]
$ echo $?
1
$ oneline="a line with Hard Disk in it"
$ [[ "${oneline}" == ~(E).*Hard\ Disk.* ]]
$ echo $?
0
$ oneline="titl this line does NOT start with title"
$ [[ "${oneline}" == ~(E)^title.* ]]
$ echo $?
1
$ oneline="title this line does start with title"
$ [[ "${oneline}" == ~(E)^title.* ]]
$ echo $?
Issue 6:
--------
135 echo ${oneline} >>$1.$$
- Use "print" and "printf" in place of "echo"
Hope this helps!
Joe
Jack Schwartz wrote:
> Hi everyone.
>
> Here is a revised webrev of code to get rid of the Hard Disk entry in
> the grub menu for USB sticks.
>
> Try as I might, I couldn't figure out a way using sed to do what I
> needed to do. So, instead I wrote a function to do it: it looks for
> lines with "Hard Disk" in them, and then deletes until either a blank
> line is seen or the word "title" appears at the beginning of a line.
> "Title" lines are kept in; blank lines are deleted.
>
> http://cr.opensolaris.org/~schwartz/081112.1/webrev/
>
> It's not a one-liner anymore, but should be straightforward enough...
>
> Please review ASAP. This code (or else it's backup 1-liner sed change)
> has to go back Friday.
>
> Thanks,
> Jack
>
>
> Jack Schwartz wrote:
>> Hi Dave.
>>
>> On 11/12/08 10:25, Dave Miner wrote:
>>> Jack Schwartz wrote:
>>>
>>>> Hi everyone.
>>>>
>>>> Please review the following one-liner that fixes accessibility in USB
>>>> stick grub menus.
>>>>
>>>> http://cr.opensolaris.org/~schwartz/081112.1/webrev
>>>>
>>>> Basically, the fix is to change from:
>>>> deleting everything beyond and including the line which says "Hard Disk"
>>>> to
>>>> deleting the grub entry for the Hard Disk (from the "Hard Disk" line
>>>> to the first blank
>>>> line afterward.
>>>>
>>>>
>>> It's correct, but fragile. Really, it should delete from a line with
>>> that title to the next title. I guess I'd take it as-is, but I'd feel
>>> better if it weren't going to require a future fix when the menu gets
>>> generated differently.
>>>
>> OK. But being that the starting file is itself an intact menu.lst for
>> CD, there shouldn't be anything accept space between menu items.
>> Ah... but what if there is no space? Now I get it...
>>
>> Here's what I'll do, so that I don't impact my other stopper (xVM
>> issue). When I get a second reviewer I'll hold this fix as a backup.
>> If I have time to develop a better fix I will and will post another
>> review. Otherwise, this one will go back. Regardless, I'll keep the
>> xVM bug (3885) as my top priority.
>>
>> Thanks,
>> Jack
>>>
>>>> I'll push as soon as I have two satisfied reviewers or 2 PM PST today,
>>>> whichever is later.
>>>>
>>>>
>>> The latter isn't an option, must have reviewers.
>>>
>> I didn't say it right. I meant that even if I had 2 reviewers I would
>> wait. But it looks like I'll wait till after 2 anyhow, since I'd now
>> like to come up with a better fix if there's time.
>>
>> Thanks,
>> Jack
>>> Dave
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ksh93_pointers.txt
URL:
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20081114/90c681f0/attachment.txt>
