Hi Joe.
Thanks so much for all of your comments, and for teaching me some new
shell tricks!
I think I'm going to go with Dave's fix, because it is more compact.
However, I'll definitely use what you've shown me in the future...
Thanks,
Jack
On 11/14/08 07:15, Joseph J VLcek wrote:
> 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
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20081114/96271f4d/attachment.html>
