ssorj commented on code in PR #369:
URL: https://github.com/apache/qpid-proton/pull/369#discussion_r863851716
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz,
sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
Review Comment:
Can we have both of these use the if/else form? I find it easier to read.
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz,
sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
+static inline void pni_class_decref(const pn_class_t *clazz, void *object) {
+ if (clazz->decref) {
+ clazz->decref(object);
+ } else {
+ pni_object_decref(object);
+ }
+}
+
+static inline int pni_class_refcount(const pn_class_t *clazz, void *object) {
+ return clazz->refcount
+ ? clazz->refcount(object)
+ : pni_object_refcount(object);
+}
+
+static inline void pni_class_free(const pn_class_t *clazz, void *object) {
Review Comment:
A bigger question about naming conventions. These are static in a library
file. Should they use pni_? In some other places, they have no prefix. (I've
long been confused about this.)
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz,
sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
+static inline void pni_class_decref(const pn_class_t *clazz, void *object) {
+ if (clazz->decref) {
+ clazz->decref(object);
+ } else {
+ pni_object_decref(object);
+ }
+}
+
+static inline int pni_class_refcount(const pn_class_t *clazz, void *object) {
+ return clazz->refcount
+ ? clazz->refcount(object)
+ : pni_object_refcount(object);
+}
+
+static inline void pni_class_free(const pn_class_t *clazz, void *object) {
+ if (clazz->free) {
+ clazz->free(object);
+ } else {
+ pni_object_free(object);
+ }
+}
+
void *pn_class_new(const pn_class_t *clazz, size_t size)
{
assert(clazz);
- void *object = clazz->newinst(clazz, size);
- if (clazz->initialize) {
+ void *object = pni_class_new(clazz, size);
+ if (object && clazz->initialize) {
clazz->initialize(object);
}
return object;
}
void *pn_class_incref(const pn_class_t *clazz, void *object)
{
- assert(clazz);
if (object) {
- clazz = clazz->reify(object);
- clazz->incref(object);
+ if (clazz==PN_OBJECT) {
Review Comment:
Here and elsewhere, I would prefer space around ==.
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz,
sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
+ pni_head(object)->refcount++;
+ }
+}
+
+static inline int pni_object_refcount(void *object)
+{
+ assert(object);
+ return pni_head(object)->refcount;
+}
+
+static inline void pni_object_decref(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ assert(head->refcount > 0);
+ head->refcount--;
+}
+
+static inline void pni_object_free(void *object)
+{
+ pni_head_t *head = pni_head(object);
+ pni_mem_deallocate(head->clazz, head);
+}
+
+void pn_object_incref(void *object) {
+ pni_object_incref(object);
+}
+
+static inline void *pni_class_new(const pn_class_t *clazz, size_t size) {
+ return clazz->newinst
+ ? clazz->newinst(clazz, size)
+ : pni_object_new(clazz, size);
+}
+
+static inline void pni_class_incref(const pn_class_t *clazz, void *object) {
+ if (clazz->incref) {
+ clazz->incref(object);
+ } else {
+ pni_object_incref(object);
+ }
+}
+
Review Comment:
Same for pni_class_refcount below.
##########
c/src/core/object/object.c:
##########
@@ -58,50 +67,130 @@ pn_cid_t pn_class_id(const pn_class_t *clazz)
return clazz->cid;
}
+static inline void *pni_object_new(const pn_class_t *clazz, size_t size)
+{
+ void *object = NULL;
+ pni_head_t *head = (pni_head_t *) pni_mem_zallocate(clazz,
sizeof(pni_head_t) + size);
+ if (head != NULL) {
+ object = head + 1;
+ head->clazz = clazz;
+ head->refcount = 1;
+ }
+ return object;
+}
+
+static inline void pni_object_incref(void *object) {
+ if (object) {
Review Comment:
Should this assert instead of null check? _refcount and _decref are
(_decref needs an explicit assert, I think). Maybe the null check in
pn_object_incref, not pni_object_incref?
In general, I'd prefer all the refcounting methods required a non-null
object, but that does require fixing up a number of places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
