Subject: Re: Status report: sysmon_cpufreq(9) + powerctl(8)
To: Juan RP <juan@xtrarom.org>
From: Iain Hibbert <plunky@rya-online.net>
List: tech-kern
Date: 10/01/2006 13:18:28
  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--0-29174030-1159701382=:1136
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII
Content-ID: <Pine.NEB.4.64.0610011244331.994@localhost.>

On Sun, 1 Oct 2006, Juan RP wrote:

> On Fri, 29 Sep 2006 20:53:38 +0100 (BST)
> Iain Hibbert <plunky@rya-online.net> wrote:
>
> > On Fri, 29 Sep 2006, Juan RP wrote:
> >
> > > Patch updated... I think I fixed all the things you said, please
> > > correct me if I'm missing something.
> >
> > When you create an object, you must release it when you dont want
> > it.. In sysmon_cpufreq_recv_data() there are some memory leaks. If
> > you add it to a dictionary (or array), the dictionary retains it but
> > you must also release it in any case.
>
> Ok... I still don't understand what functions retain the objects but I
> fixed most of them.

If you create or explicitly retain an object then you must release it when
you no longer keep a reference. I dont think anything secretly retains an
object - when you do a *_get you only get a reference to the object and if
you want to keep it then you should retain it.

You seem to be getting the hang of it in any case, I attached some
comments but in general

I'm not sure for in kernel use if its necessary to check the result of
prop_*_create() routines. So far as I know, they use WAITOK and can't fail
but it doesnt specify that in the manpage. Jason?

Ditto for prop_dictionary_set() but that could fail if the dictionary was
immutable I guess, so no need to check if its your dictionary but if you
got it elsewhere then maybe you do (not sure who owns dv_properties
dictionaries and if it would be acceptable to consider it mutable without
checking)

in powerctl.c/set_newfreq() there are some unreleased objects. I dont
think it matters in a single pass program especially, but I see Coverity
will tag memory leaks anyway.. I would do it this way:

	obj = prop_string_create_cstring("foo");
	if (obj == NULL || !prop_dictionary_set(dict, "key", obj))
		err(EXIT_FAILURE, "key");

	prop_object_release(obj);

iain
--0-29174030-1159701382=:1136
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII; NAME=cpufreq.diff
Content-Transfer-Encoding: BASE64
Content-ID: <Pine.NEB.4.64.0610011216220.1136@localhost.>
Content-Description: cpufreq.diff comments
Content-Disposition: ATTACHMENT; FILENAME=cpufreq.diff

KwkJTElTVF9GT1JFQUNIKGxzbWNmLCAmc3lzbW9uX2NwdWZyZXFfbGlzdCwg
c21jZl9saXN0KQ0KKwkJCWlmIChwcm9wX3N0cmluZ19lcXVhbHNfY3N0cmlu
ZyhteXN0ciwgbHNtY2YtPnNtY2ZfbmFtZSkpDQorCQkJCWJyZWFrOyAvKiBm
b3VuZCAqLw0KKw0KKwkJaWYgKGxzbWNmID09IE5VTEwpIHsNCisJCQllcnJv
ciA9IEVJTlZBTDsNCg0KcmVsZWFzZSB1ZGljdA0KDQorCQlzZGljdCA9IHBy
b3BfZGljdGlvbmFyeV9nZXQoZGV2aWNlX3Byb3BlcnRpZXMoY2ktPmNpX2Rl
diksDQorCQkJCQlwcm9wX3N0cmluZ19jc3RyaW5nX25vY29weShteXN0cikp
Ow0KKwkJaWYgKHByb3Bfb2JqZWN0X3R5cGUoc2RpY3QpICE9IFBST1BfVFlQ
RV9ESUNUSU9OQVJZKSB7DQorCQkJRFBSSU5URigoIiVzOiBzZGljdCBub3Qg
YWNjZXB0ZWQuXG4iLCBfX2Z1bmNfXykpOw0KKwkJCXByb3Bfb2JqZWN0X3Jl
bGVhc2UodWRpY3QpOw0KKwkJCWVycm9yID0gRU9QTk9UU1VQUDsNCisJCQli
cmVhazsNCisJCX0NCisNCisJCWlmIChzZGljdCA9PSBOVUxMKSB7DQorCQkJ
ZXJyb3IgPSBFSU5WQUw7DQorCQkJYnJlYWs7DQorCQl9DQoNCnRoaXMgY2hl
Y2sgaXMgbm93IHJlZHVuZGFudCBzaW5jZSAocHJvcF9vYmplY3RfdHlwZShO
VUxMKSAhPSBQUk9QX1RZUEVfRElDVElPTkFSWSkNCg0KKwkJLyoNCisJCSAq
IENhbGwgdGhlIGFwcHJvcGlhdGUgZnVuY3Rpb24gdmlhIHNtY2YtPnNtY2Zf
dHlwZS4NCisJCSAqLw0KKwkJc3dpdGNoIChsc21jZi0+c21jZl90eXBlKSB7
DQorCQljYXNlIENQVUZSRVFfVFlQRV9MSVNUOg0KKwkJCURQUklOVEYoKCIl
czogbGlzdF9zZXRmcmVxIGNhc2UuXG4iLCBfX2Z1bmNfXykpOw0KKwkJCWVy
cm9yID0gc3lzbW9uX2NwdWZyZXFfbGlzdF9zZXRmcmVxKGxzbWNmLA0KKwkJ
CQkJCQkgICAgJnNkaWN0LA0KKwkJCQkJCQkgICAgdWRpY3QpOw0KKwkJCWJy
ZWFrOw0KKwkJZGVmYXVsdDoNCisJCQllcnJvciA9IEVPUE5PVFNVUFA7DQor
CQkJYnJlYWs7DQorCQl9DQorDQorCQlEUFJJTlRGKCgiJXM6IHNldGZyZXEg
ZXJyb3I9JWRcbiIsIF9fZnVuY19fLCBlcnJvcikpOw0KKw0KKwkJaWYgKGVy
cm9yKQ0KKwkJCWJyZWFrOw0KDQpyZWxlYXNlIHVkaWN0DQoNCltzeXNtb25f
Y3B1ZnJlcV9yZWN2X2RhdGFdDQorCURQUklOVEYoKCIlczogZXJyb3I9JWRc
biIsIF9fZnVuY19fLCBlcnJvcikpOw0KKwlpZiAoZXJyb3IpDQorCQlyZXR1
cm4gZXJyb3I7DQoNCnJlbGVhc2UgZGljdA0KDQorc3RhdGljIGludA0KK3N5
c21vbl9jcHVmcmVxX21rX2xpc3RfZGljdGlvbmFyeShzdHJ1Y3Qgc3lzbW9u
X2NwdWZyZXEgKnNtY2YsDQorCQkJCSAgcHJvcF9kaWN0aW9uYXJ5X3QgKmRp
Y3QpDQoNCmNhbiBwYXNzIHByb3BfZGljdGlvbmFyeV90IHJhdGhlciB0aGFu
ICoNCg0KKwlhcnJheSA9IHByb3BfYXJyYXlfY3JlYXRlKCk7DQorCWlmIChh
cnJheSA9PSBOVUxMKSB7DQorCQlwcm9wX29iamVjdF9yZWxlYXNlKG9iaik7
DQorCQlyZXR1cm4gRUlOVkFMOw0KKwl9DQoNCm9iaiB3YXMgaXMgYWxyZWFk
eSByZWxlYXNlZA0KDQorc3RhdGljIGludA0KK3N5c21vbl9jcHVmcmVxX2xp
c3Rfc2V0ZnJlcShzdHJ1Y3Qgc3lzbW9uX2NwdWZyZXEgKmxzbWNmLA0KKwkJ
CSAgICBwcm9wX29iamVjdF90ICprZGljdCwNCisJCQkgICAgcHJvcF9kaWN0
aW9uYXJ5X3QgdWRpY3QpDQorew0KKwljcHVmcmVxX2RhdGFfdCBjZnJlcTsN
CisJcHJvcF9vYmplY3RfdCBvYmo7DQorCWludCBpLCBteWNudDsNCisJdWlu
dDE2X3QgbmV3ZnJlcTsNCisJYm9vbGVhbl90IGZyZXFmb3VuZDsNCisNCisJ
ZnJlcWZvdW5kID0gRkFMU0U7DQorDQorCW1lbXNldCgmY2ZyZXEsIDAsIHNp
emVvZihjZnJlcSkpOw0KKw0KKwkvKg0KKwkgKiBQYXJzZSAiY3B1ZnJlcS1u
ZXctZnJlcXVlbmN5IiBrZXkgdG8gb2J0YWluIHRoZSB2YWx1ZS4NCisJICov
DQorCW9iaiA9IHByb3BfZGljdGlvbmFyeV9nZXQodWRpY3QsICJjcHVmcmVx
LW5ldy1mcmVxdWVuY3kiKTsNCisJaWYgKHByb3Bfb2JqZWN0X3R5cGUob2Jq
KSAhPSBQUk9QX1RZUEVfTlVNQkVSKSB7DQorCQlEUFJJTlRGKCgiJXM6IG9i
aiBub3QgYWNjZXB0ZWQuXG4iLCBfX2Z1bmNfXykpOw0KKwkJcmV0dXJuIEVP
UE5PVFNVUFA7DQorCX0NCisNCisJbmV3ZnJlcSA9IHByb3BfbnVtYmVyX2lu
dGVnZXJfdmFsdWUob2JqKTsNCisNCisJRFBSSU5URigoIiVzOiBuZXdmcmVx
OiAlZCBjZnJlcV90LT5jZl9jdXJmcmVxOiAlZFxuIiwNCisJICAgIF9fZnVu
Y19fLCAodW5zaWduZWQpbmV3ZnJlcSwNCisJICAgICh1bnNpZ25lZClsc21j
Zi0+Y2ZyZXFfZGF0YV90LT5jZl9jdXJmcmVxKSk7DQorDQorCS8qDQorCSAq
IEdldCBjdXJyZW50IGxpc3Qgb2YgZnJlcXVlbmNpZXMuDQorCSAqLw0KKwlv
YmogPSBwcm9wX2RpY3Rpb25hcnlfZ2V0KCprZGljdCwgImxpc3Qtb2YtZnJl
cXVlbmNpZXMiKTsNCisJaWYgKHByb3Bfb2JqZWN0X3R5cGUob2JqKSAhPSBQ
Uk9QX1RZUEVfQVJSQVkpIHsNCisJCURQUklOVEYoKCIlczogb2JqOiAlZFxu
IiwgX19mdW5jX18sIHByb3Bfb2JqZWN0X3R5cGUob2JqKSkpOw0KKwkJcmV0
dXJuIEVPUE5PVFNVUFA7DQorCX0NCisNCisJbXljbnQgPSBwcm9wX2FycmF5
X2NvdW50KG9iaik7DQorCURQUklOVEYoKCIlczogbXljbnQ6ICVkXG4iLCBf
X2Z1bmNfXywgbXljbnQpKTsNCg0KYnR3IGZvciBib3RoIG9mIHRoZXNlIHlv
dSBjb3VsZCBqdXN0IGlnbm9yZSB0aGUgaW52YWxpZCB0eXBlcy4gQm90aA0K
bmV3ZnJlcSBhbmQgbXljbnQgd2lsbCBiZSAwIGlmIHRoZSBvYmplY3QgaXMg
bWlzc2luZyBvciBpbnZhbGlkLCBzbyANCnVubGVzcyAwIGlzIGEgdmFsaWQg
ZnJlcXVlbmN5LCBpdCB3b250IGJlIGZvdW5kLg0K

--0-29174030-1159701382=:1136
Content-Type: TEXT/PLAIN; charset=US-ASCII; name=powerctl.c
Content-Transfer-Encoding: BASE64
Content-ID: <Pine.NEB.4.64.0610011318280.994@localhost.>
Content-Description: powerctl.c comments
Content-Disposition: attachment; filename=powerctl.c

c3RhdGljIHZvaWQNCnRha2VfZGljdGlvbmFyeV9mcm9tX2tlcm5lbChpbnQg
KmZkcCwgaW50IHJhd19vdXRwdXQsIGludCBwcmludF9mcmVxKQ0Kew0KCXBy
b3BfZGljdGlvbmFyeV90IGRpY3Q7DQoJcHJvcF9vYmplY3RfaXRlcmF0b3Jf
dCBpdGVyOw0KCXByb3Bfb2JqZWN0X3Qgb2JqOw0KCWludCBpLCBydmFsOw0K
DQoJbXlyZXMuZnJlcV9hcnJheV9jbnQgPSAwOw0KDQoJbWVtc2V0KCZteXJl
cywgMCwgc2l6ZW9mKG15cmVzKSk7DQoNCgkvKg0KCSAqIFRha2UgZGljdGlv
bmFyeSBmcm9tIGtlcm5lbC4NCgkgKi8NCglydmFsID0gcHJvcF9kaWN0aW9u
YXJ5X3JlY3ZfaW9jdGwoKmZkcCwgU01DUFVGUkVRX0dESUNULCAmZGljdCk7
DQoJaWYgKHJ2YWwpDQoJCWVycihFWElUX0ZBSUxVUkUsICJwcm9wX2RpY3Rp
b25hcnlfcmVjdl9pb2N0bCIpOw0KDQoJaWYgKHJhd19vdXRwdXQpIHsNCgkJ
cHJpbnRmKCIlc1xuIiwgcHJvcF9kaWN0aW9uYXJ5X2V4dGVybmFsaXplKGRp
Y3QpKTsNCgkJcmV0dXJuOw0KCX0NCg0KCS8qDQoJICogQ3JlYXRlIGFuIGl0
ZXJhdG9yIHRvIGl0ZXJhdGUgb3ZlciBhbGwgaXRlbXMuDQoJICovDQoJaXRl
ciA9IHByb3BfZGljdGlvbmFyeV9pdGVyYXRvcihkaWN0KTsNCglmb3IgKGkg
PSAwOyAob2JqID0gcHJvcF9vYmplY3RfaXRlcmF0b3JfbmV4dChpdGVyKSkg
IT0gTlVMTDsgaSsrKSB7DQoJCXByb3Bfb2JqZWN0X2l0ZXJhdG9yX3Qgc2l0
ZXI7DQoJCXByb3Bfb2JqZWN0X3Qgc2RpY3Q7DQoJCWludCBqOw0KDQoJCS8q
DQoJCSAqIGRydm5hbWUgbm93IGNvbnRhaW5zIHRoZSBrZXkgc3ltYm9sIG9m
IHRoZSBmaXJzdA0KCQkgKiBkaWN0aW9uYXJ5Lg0KCQkgKi8NCgkJbXlyZXMu
ZHJ2bmFtZSA9IHByb3BfZGljdGlvbmFyeV9rZXlzeW1fY3N0cmluZ19ub2Nv
cHkob2JqKTsNCg0KCQkvKg0KCQkgKiBUYWtlIHRoZSBzZWNvbmQgZGljdGlv
bmFyeS4NCgkJICovDQoJCXNkaWN0ID0gcHJvcF9kaWN0aW9uYXJ5X2dldChk
aWN0LCBteXJlcy5kcnZuYW1lKTsNCg0KKioqKg0KV2hpbGUgdGhpcyBpcyB2
YWxpZCwgSSd2ZSBiZWVuIHRyeWluZyB0byB3b3JrIG91dCB3aGF0IHlvdSBh
cmUgd2FudGluZw0KZm9yIHNvbWUgbWludXRlcy4uIEkgdGhpbmsNCg0KCQlz
ZGljdCA9IHByb3BfZGljdGlvbmFyeV9nZXRfa2V5c3ltKGRpY3QsIG9iaik7
DQoNCmlzIHdoYXQgeW91IHdhbnRlZD8NCioqKioNCg0KCQlpZiAocHJvcF9v
YmplY3RfdHlwZShzZGljdCkgIT0gUFJPUF9UWVBFX0RJQ1RJT05BUlkpIHsN
CgkJCXByb3Bfb2JqZWN0X2l0ZXJhdG9yX3JlbGVhc2UoaXRlcik7DQoJCQlw
cm9wX29iamVjdF9yZWxlYXNlKGRpY3QpOw0KCQkJZXJyKEVYSVRfRkFJTFVS
RSwgIm15cmVzLmRydm5hbWUiKTsNCgkJfQ0KDQoJCS8qIA0KCQkgKiBJdGVy
YXRlIG92ZXIgdGhlIHNlY29uZCBkaWN0aW9uYXJ5Lg0KCQkgKi8NCgkJc2l0
ZXIgPSBwcm9wX2RpY3Rpb25hcnlfaXRlcmF0b3Ioc2RpY3QpOw0KDQoqKioq
DQpJIGRvbnQgdGhpbmsgeW91IG5lZWQgdG8gaXRlcmF0ZSB0aGlzIGRpY3Rp
b25hcnksIGVhY2ggaXRlcmF0aW9uIGRvZXMNCnRoZSBzYW1lIHRoaW5nIC4u
Pw0KKioqKg0KDQoJCWZvciAoaiA9IDA7DQoJCSAgICAob2JqID0gcHJvcF9v
YmplY3RfaXRlcmF0b3JfbmV4dChzaXRlcikpICE9IE5VTEw7IGorKykgew0K
CQkJcHJvcF9udW1iZXJfdCBudW07DQoJCQlwcm9wX29iamVjdF90IG15b2Jq
Ow0KCQkJcHJvcF9vYmplY3RfaXRlcmF0b3JfdCBteWl0ZXI7DQoJCQlpbnQg
ZjsNCg0KCQkJLyoNCgkJCSAqIEdldCBjdXJyZW50IGZyZXF1ZW5jeS4NCgkJ
CSAqLw0KCQkJbnVtID0gcHJvcF9kaWN0aW9uYXJ5X2dldChzZGljdCwgImN1
cnJlbnQtZnJlcXVlbmN5Iik7DQoJCQlpZiAocHJvcF9vYmplY3RfdHlwZShu
dW0pICE9IFBST1BfVFlQRV9OVU1CRVIpDQoJCQkJYnJlYWs7DQoNCgkJCW15
cmVzLmN1cmZyZXEgPSBwcm9wX251bWJlcl9pbnRlZ2VyX3ZhbHVlKG51bSk7
DQoNCgkJCS8qIA0KCQkJICogR2V0IGN1cnJlbnQgdm9sdGFnZS4NCgkJCSAq
Lw0KCQkJbnVtID0gcHJvcF9kaWN0aW9uYXJ5X2dldChzZGljdCwgImN1cnJl
bnQtdm9sdGFnZSIpOw0KCQkJaWYgKHByb3Bfb2JqZWN0X3R5cGUobnVtKSAh
PSBQUk9QX1RZUEVfTlVNQkVSKQ0KCQkJCWJyZWFrOw0KDQoJCQlteXJlcy5j
dXJ2b2x0ID0gcHJvcF9udW1iZXJfaW50ZWdlcl92YWx1ZShudW0pOw0KDQoJ
CQkvKg0KCQkJICogR2V0IGxpc3Qgb2YgZnJlcXVlbmNpZXMuDQoJCQkgKi8N
CgkJCW15b2JqID0NCgkJCSAgICBwcm9wX2RpY3Rpb25hcnlfZ2V0KHNkaWN0
LCAibGlzdC1vZi1mcmVxdWVuY2llcyIpOw0KCQkJbXlpdGVyID0gcHJvcF9h
cnJheV9pdGVyYXRvcihteW9iaik7DQoJCQlteXJlcy5mcmVxX2FycmF5X2Nu
dCA9IHByb3BfYXJyYXlfY291bnQobXlvYmopOw0KDQoJCQlmb3IgKGYgPSAw
Ow0KCQkJICAgIChteW9iaiA9IHByb3Bfb2JqZWN0X2l0ZXJhdG9yX25leHQo
bXlpdGVyKSkgIT0gTlVMTDsNCgkJCSAgICBmKyspIHsNCgkJCQlteXJlcy5m
cmVxbGlzdFtmXSA9DQoJCQkJICAgIHByb3BfbnVtYmVyX2ludGVnZXJfdmFs
dWUobXlvYmopOw0KCQkJfQ0KDQoJCQlwcm9wX29iamVjdF9pdGVyYXRvcl9y
ZWxlYXNlKG15aXRlcik7DQoJCX0NCg0KCQlwcm9wX29iamVjdF9pdGVyYXRv
cl9yZWxlYXNlKHNpdGVyKTsNCg0KCQlpZiAocHJpbnRfZnJlcSkNCgkJCXNo
b3dfZnJlcXMoKTsNCgl9DQoNCglwcm9wX29iamVjdF9pdGVyYXRvcl9yZWxl
YXNlKGl0ZXIpOw0KCXByb3Bfb2JqZWN0X3JlbGVhc2UoZGljdCk7DQp9DQo=

--0-29174030-1159701382=:1136--