Quoth Liane Praza on Wed, Sep 10, 2008 at 10:44:33AM -0700:
> http://cr.opensolaris.org/~lianep/webrev-20080910
lib/libscf/common/scf_tmpl.c
529: Please include a note saying that the function skips over
zero-length and duplicate values. And maybe that the
duplicate-detection algorithm won't scale very well.
570-1: cstyle says comments longer than one line should be block
comments, formatted like
/*
* the array...
* we will...
*/
606: cstyle calls for a blank line after local declarations.
759: "usr_type" -> "use_type".
859: Doesn't this function return the name of a property group? If
so, then "Returns the name of the property template prop" is
misleading.
scf_tmpl_get_by_prop(): Shouldn't this fail if any flags are set?
lib/libscf/inc/libscf.h
100: I think most people would interpret this to mean that consumers
should provide functions which set and populate the structure.
I think you mean to say that functions which set or populate the
structure assume that it is either uninitialized or destroyed.
cmd/svc/svccfg/svccfg_help.c
34-5: I think you should qualify these sentences to when they happen.
Something like, "For a manifest file, process without changing the
repository.\nFor an instance FMRI, validate against the template
specifications."
cmd/svc/svccfg/svccfg_internal.c
1242: Does load_pg() set scf_error()? I expected you to pass rc
here.
cmd/svc/svccfg/svccfg_libscf.c
9439: Is there a reason this isn't semerr(gettext("..."))?
10897-8: I'm not sure what ", describe -v" or ", describe -t" mean.
Do they mean that they should only occur during the execution of
those commands? And can't templates == 1? Does that have no effect
on this function? I think a little more explanation would be
helpful here.
11046: Is there a reason not to use for (i = 0;
i < values.value_count; ++i) here?
11301: Why is it necessary to cast -1 to (ssize_t)?
11380: cstyle says comments more than one line long should be in the
block comment format.
11395: Couldn't we hit this if the property was the wrong type? If
so, it doesn't seem appropriate to die.
13676: cstyle says comments more than one line long should be in block
comment format.
cmd/svc/svccfg/svccfg_tmpl.c
30: Excellent comment.
1805: It seems that im_tmpl_error_create() has been eliminated. In
that case, it should be removed from this comment, too.
cmd/svc/svccfg/svccfg_xml.c
1323: Can we say by how many characters too long the name was? Or
maybe the maximum length? Or maybe both?
1471: I think we should say that it's not a legal number, or
something.
1474,1517,1698,1905,2295,2300,2560,2607,2620: Is it a ok to continue
if semerr() returns?
David
