LGTM.
Antonello
On 03/18/10 10:16 AM, Sean Wilcox wrote:
> On 03/17/10 04:23 PM, Sean Wilcox wrote:
>> A couple of comments on the notes below...
>>
>> On 03/17/10 03:58 PM, Antonello Cruz wrote:
>>> Please find my feedback for usr/src/cmd/svc/milestone/manifest-import
>>> below.
>>>
>>> usr/src/cmd/svc/milestone/manifest-import
>>> 61: if you are looking for the prorpety manifestfile, why not
>>> simplify by doing
>>>
>>> smfmfiles=`/usr/bin/svcprop smf/manifest | \
>>> awk '/\/manifestfile / {print $3}' | grep '^/lib'`
>>>
>>> You could probably make awk match the '^/lib' in the same
>>> command, but you'd have to ask barts how to do it ;-)
>>>
>>> Additionally, why are you using egrep at the end of the pipe in
>>> the original code?
>>>
>>> 64: we can use a similar simplification as above
>>>
>> 64: we can use a similar simplification as above
>>
>> Not sure I remember why we used egrep there... but can definitely make
>> the
>> change to the awk to get the manifestfile keyword. As I look at this what
>> about a case where a manifest file is named something like
>> foo_manifestfile.xml
>> could that trip this up? Need to investigate this and harden this code
>> section
>> a bit.
>>
>>
>
> I did a bit of playing around with this, this morning it looks like the
> simplification could be the following :
>
> function cleanup_needwork {
> if [ "$early" == true ]; then
> smfmfiles=`/usr/bin/svcprop smf/manifest | \
> awk '(/^lib_/) && (/\/manifestfile /) {print $3}'`
> else
> smfmfiles=`/usr/bin/svcprop smf/manifest | \
> awk '/\/manifestfile / {print $3}'`
> fi
>
> nw=`/lib/svc/bin/mfstscan $smfmfiles 2>&1 1>/dev/null`
> [ "$nw" ] && return 1
>
> return 0
> }
>
> And checking for the specific property with the '/manifestfile ' makes
> sure we get the right thing. Unless anybody can
> see any holes I'll update the code.
>
>
