Jack,
In an attempt to help ( and since I reviewed this late... ;)
I am attaching what I think is a better version of function
strip_grub_hd_entry.
I have "not" tested it.
Hope this helps!
Joe
Joseph J VLcek wrote:
>
> 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
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> 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: temp
URL:
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20081114/bffa41a8/attachment.ksh>
