Hi Joe,
On 06/10/10 12:32 PM, [email protected] wrote:
[...]
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
Issue:
------
"Not all the copyrights are the same?"
To be honest, I am not sure what the issue is. 'hg pbchk'
does not complain, but I can see there might be some discrepancies
I am not aware of. Could you please elaborate more on this ?
General:
I just noticed differences in the year ranges. Maybe this is OK. Here
are a few examples:
usr/src/cmd/Makefile.cmd
23 # Copyright (c) 2007, 2010, Oracle and/or its affiliates. All
rights reserved.
usr/src/cmd/auto-install/auto_install.h
22 * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All
rights reserved.
usr/src/cmd/system-config/svc/svc-system-config
23 # Copyright (c) 2010, Oracle and/or its affiliates. All rights
reserved.
I see. Thank you for clarifying this.
The first year indicates when the file was created (added to the gate),
the second year when it was last modified. If both are the same,
year is listed only once. See following Heads-up for more details:
http://static.opensolaris.org/on/flagdays/pages/20100324114440.html
Issue:
------
"I don't believe I am suggesting this as I usually like to break
code up into functions but Lines 105 -> 145 are used to encase
a single line of code into 2 different functions.
Why are the functions necessary?"
Why not change from:
342 ! $(astring_prop_configured "$expire") && return
To:
if [[ "${expire}" != "${SMF_DEF_ASTRING}" ]]; then return; fi
I believe having them separated in function improves readability
and maintainability of the code. The similar reason why
macros are used in C code.
"This would eliminate 40 lines of code from this file."
Well, the most of those 40 lines is only comment section
explaining what those small functions do.
I would prefer to leave the code as is, but if you would rather
change it, I will do that. Please let me know.
[...]
I see the value in your perspective too. It's fine with me to leave it
as you have it. Thank you.
Thank you. Based on this, I will leave it as is.
Issue/Suggestion:
-----------------
"ksh93 has built in variable/parameter expansion formats."
For example:
182 prop_value=$(print $prop_value | $SED -e 's/\\\(.\)/\1/g')
Could be done with:
prop_value=${prop_value//\//}
Unfortunately, it does not seem to work for quoted backslashes, i.e.
we want to replace '\\' with '\',but the example above removes
them all.
[...]
Ah yes. I believe an extra "\/" in the match string should do the
trick. Does this work for what you need?
e.g.:
$ prop_value="aaa//bbb/ccc//ddd"
$ print ${prop_value//\/\//}
aaabbb/cccddd
Unfortunately, it is still not what we would need. In general, we
need to replace sequence of '<backslash><char>' with '<char>' where
<char> can be any of:
';', '&', '(', ')', '|', '^', '<', '>', newline, space, tab, backslash,
'"', single-quote, '`'
In particular, backslash complicates things, since
it serves as both quoted as well as quoting character.
Issue/Suggestion:
-----------------
"Using Parameter Expansion would reduce the difference between
functions"
161 get_astring_prop()
and
207 get_count_prop()
"Which could allow them to be more easily combined into a single
fuction as the logic in both is very similar."
To be honest, I do not see how Parameter expansion might be
used in this case - could you please elaborate more on that ?
[...]
What I am thinking is if the Parameter expansion could be made to
work in this case Lines 181 -> 186 would be collapsed to a single
line. This would result in get_astring_prop() and get_count_prop() to
be identical except for a couple of different lines. Which would make
them a good candidate to collapse into a single function.
I see. Even if the parameter expansion does not work,
I have tried to consolidate those functions into one -
please see updated webrev.
New usr/src/cmd/system-config/tools/sc_conv.ksh 276 lines changed
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
[...]
Issue:
------
"Count the NAWK code on lines 207 -> 224 be place in a function?
Then it coule be invoked with, for example:"
get_value(username); username=$?
get_value(userpass); userpass=$?
get_value(description); description=$?
get_value(rootpass); rootpass=$?
get_value(timezone); timezone=$?
get_value(nodename); nodename=$?
"If it is difficult to pass an argument to the NAWK command for the
string to match perhaps this wouldn't be a good idea, but if it
could be done the code would be more readable."
To be honest, I have not found how to expand variable as part
of regular expression in nawk, but I am open to suggestions, since I
agree
this is candidate to be moved to function.
[...]
I don't know how either. I was hoping you might know! ;)
To tell the truth awk is not my strong point :-)
But I consulted with 'awk & sed' book written by Helmut Herold
and found out that internal match() string function could
take care of this :-)
Please see updated webrev for result:
http://cr.opensolaris.org/~dambi/bug-15723-cr/
delta webrev:
http://cr.opensolaris.org/~dambi/bug-15723-cr-delta/
Thank you !
Jan
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
