Great Jack,
Glad I could provide "some... ? ;)" help.
Joe
Jack Schwartz wrote:
> 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
>
