[1/2] libpdbg: Create struct pdbg_target_priv
diff mbox series

Message ID 20190903075934.3525-1-alistair@popple.id.au
State New
Headers show
Series
  • [1/2] libpdbg: Create struct pdbg_target_priv
Related show

Checks

Context Check Description
snowpatch_ozlabs/build-multiarch success Test build-multiarch on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (e005e0863a44f04c931b09804f258fb1c0b6f14c)

Commit Message

Alistair Popple Sept. 3, 2019, 7:59 a.m. UTC
In order to allow external programs to register their own hardware
units it is required to expose the definition of struct
pdbg_target. However this means the struct definition now becomes part
of the library ABI/API, making it difficult to change fields without
breaking applications.

It also means we would have to install the ccan list headers, which
themselves depend on other ccan modules. Instead create a seperate
struct definition (struct pdbg_target_priv) which encapsulates the
libpdbg specific fields which should not be accessed directly by
applications and switch the library to use those.

This should allow a stable definition of struct pdbg_target to be
exported to applications without preventing changes to the underlying
library fields.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 libpdbg/device.c  | 95 +++++++++++++++++++++++++++--------------------
 libpdbg/hwunit.c  |  3 ++
 libpdbg/libpdbg.c | 30 +++++++++------
 libpdbg/target.c  | 12 +++---
 libpdbg/target.h  |  8 +++-
 5 files changed, 89 insertions(+), 59 deletions(-)

Comments

Amitay Isaacs Sept. 4, 2019, 3:30 a.m. UTC | #1
On Tue, 2019-09-03 at 17:59 +1000, Alistair Popple wrote:
> In order to allow external programs to register their own hardware
> units it is required to expose the definition of struct
> pdbg_target. However this means the struct definition now becomes
> part
> of the library ABI/API, making it difficult to change fields without
> breaking applications.
> 
> It also means we would have to install the ccan list headers, which
> themselves depend on other ccan modules. Instead create a seperate
> struct definition (struct pdbg_target_priv) which encapsulates the
> libpdbg specific fields which should not be accessed directly by
> applications and switch the library to use those.
> 
> This should allow a stable definition of struct pdbg_target to be
> exported to applications without preventing changes to the underlying
> library fields.

I agree with the idea, but I have few reservations about naming.

Even though internally libpdbg uses pdbg_target to store part of
hardware unit, exposing pdbg_target as a hardware unit is a bad idea. 
From libpdbg api it's clear that pdbg_target represents an instance of
a hardware unit.  So overloading pdbg_target as a hardware unit adds
more confusion.

It would make more sense (to me at least) if we call new structure
pdbg_hwunit (or similar but different from pdbg_target).  This will
make the libpdbg api cleaner and less confusing.  (What we really need
is virtual base class and every hardware unit is derived from that. 
Need a good way to do that in C.)

Also, let's not add any structures to libpdbg API without pdbg_ prefix
(e.g. hw_unit_info).  I guess this will involve a bit of churn through
libpdbg, but in the long run it will be better.

Amitay. 
 
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  libpdbg/device.c  | 95 +++++++++++++++++++++++++++----------------
> ----
>  libpdbg/hwunit.c  |  3 ++
>  libpdbg/libpdbg.c | 30 +++++++++------
>  libpdbg/target.c  | 12 +++---
>  libpdbg/target.h  |  8 +++-
>  5 files changed, 89 insertions(+), 59 deletions(-)
> 
> diff --git a/libpdbg/device.c b/libpdbg/device.c
> index efa9ce4..e009a1a 100644
> --- a/libpdbg/device.c
> +++ b/libpdbg/device.c
> @@ -67,6 +67,7 @@ static const char *take_name(const char *name)
>  /* Adds information representing an actual target */
>  static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int
> node_offset)
>  {
> +	struct pdbg_target_priv *target_priv;
>  	struct pdbg_target *target;
>  	struct pdbg_target_class *target_class;
>  	const struct hw_unit_info *hw_info = NULL;
> @@ -98,19 +99,21 @@ static struct pdbg_target
> *dt_pdbg_target_new(const void *fdt, int node_offset)
>  		/* Couldn't find anything implementing this target */
>  		return NULL;
>  
> -	target = calloc(1, size);
> -	if (!target) {
> +	target_priv = calloc(1, sizeof(struct pdbg_target_priv) -
> sizeof(struct pdbg_target) + size);
> +	if (!target_priv) {
>  		prerror("Failed to allocate node\n");
>  		abort();
>  	}
>  
> +	target = &target_priv->target;
> +
>  	/* hw_info->hw_unit points to a per-target struct type. This
>  	 * works because the first member in the per-target struct is
>  	 * guaranteed to be the struct pdbg_target (see the comment
>  	 * above DECLARE_HW_UNIT). */
>  	memcpy(target, hw_info->hw_unit, size);
>  	target_class = get_target_class(target);
> -	list_add_tail(&target_class->targets, &target->class_link);
> +	list_add_tail(&target_class->targets, &target_priv-
> >class_link);
>  
>  	return target;
>  }
> @@ -118,31 +121,36 @@ static struct pdbg_target
> *dt_pdbg_target_new(const void *fdt, int node_offset)
>  static struct pdbg_target *dt_new_node(const char *name, const void
> *fdt, int node_offset)
>  {
>  	struct pdbg_target *node = NULL;
> -	size_t size = sizeof(*node);
> +	struct pdbg_target_priv *target_priv;
>  
> -	if (fdt)
> +	if (fdt) {
>  		node = dt_pdbg_target_new(fdt, node_offset);
> -
> -	if (!node)
> -		node = calloc(1, size);
> +		target_priv = pdbg_target_to_priv(node);
> +	}
>  
>  	if (!node) {
> +		target_priv = calloc(1, sizeof(*target_priv));
> +		node = &target_priv->target;
> +	}
> +
> +	if (!node && !target_priv) {
>  		prerror("Failed to allocate node\n");
>  		abort();
>  	}
>  
> -	node->dn_name = take_name(name);
> +	target_priv->dn_name = take_name(name);
>  	node->parent = NULL;
> -	list_head_init(&node->properties);
> -	list_head_init(&node->children);
> -	node->phandle = ++last_phandle;
> +	list_head_init(&target_priv->properties);
> +	list_head_init(&target_priv->children);
> +	target_priv->phandle = ++last_phandle;
>  
>  	return node;
>  }
>  
>  static const char *get_unitname(const struct pdbg_target *node)
>  {
> -	const char *c = strchr(node->dn_name, '@');
> +	struct pdbg_target_priv *target_priv =
> pdbg_target_to_priv(node);
> +	const char *c = strchr(target_priv->dn_name, '@');
>  
>  	if (!c)
>  		return NULL;
> @@ -152,13 +160,14 @@ static const char *get_unitname(const struct
> pdbg_target *node)
>  
>  static int dt_cmp_subnodes(const struct pdbg_target *a, const struct
> pdbg_target *b)
>  {
> +	struct pdbg_target_priv *priv_a = pdbg_target_to_priv(a),
> *priv_b = pdbg_target_to_priv(b);
>  	const char *a_unit = get_unitname(a);
>  	const char *b_unit = get_unitname(b);
>  
> -	ptrdiff_t basenamelen = a_unit - a->dn_name;
> +	ptrdiff_t basenamelen = a_unit - priv_a->dn_name;
>  
>  	/* sort hex unit addresses by number */
> -	if (a_unit && b_unit && !strncmp(a->dn_name, b->dn_name,
> basenamelen)) {
> +	if (a_unit && b_unit && !strncmp(priv_a->dn_name, priv_b-
> >dn_name, basenamelen)) {
>  		unsigned long long a_num, b_num;
>  		char *a_end, *b_end;
>  
> @@ -170,29 +179,31 @@ static int dt_cmp_subnodes(const struct
> pdbg_target *a, const struct pdbg_target
>  			return (a_num > b_num) - (a_num < b_num);
>  	}
>  
> -	return strcmp(a->dn_name, b->dn_name);
> +	return strcmp(priv_a->dn_name, priv_b->dn_name);
>  }
>  
>  static bool dt_attach_root(struct pdbg_target *parent, struct
> pdbg_target *root)
>  {
> -	struct pdbg_target *node = NULL;
> +	struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root);
> +	struct pdbg_target_priv *parent_priv =
> pdbg_target_to_priv(parent);
> +	struct pdbg_target_priv *node = NULL;
>  
>  	assert(!root->parent);
>  
> -	if (list_empty(&parent->children)) {
> -		list_add(&parent->children, &root->list);
> +	if (list_empty(&parent_priv->children)) {
> +		list_add(&parent_priv->children, &root_priv->list);
>  		root->parent = parent;
>  
>  		return true;
>  	}
>  
> -	dt_for_each_child(parent, node) {
> -		int cmp = dt_cmp_subnodes(node, root);
> +	dt_for_each_child(parent_priv, node) {
> +		int cmp = dt_cmp_subnodes(&node->target, root);
>  
>  		/* Look for duplicates */
>  		if (cmp == 0) {
>  			prerror("DT: %s failed, duplicate %s\n",
> -				__func__, root->dn_name);
> +				__func__, root_priv->dn_name);
>  			return false;
>  		}
>  
> @@ -202,7 +213,7 @@ static bool dt_attach_root(struct pdbg_target
> *parent, struct pdbg_target *root)
>  			break;
>  	}
>  
> -	list_add_before(&parent->children, &root->list, &node->list);
> +	list_add_before(&parent_priv->children, &root_priv->list,
> &node->list);
>  	root->parent = parent;
>  
>  	return true;
> @@ -219,7 +230,7 @@ static char *dt_get_path(const struct pdbg_target
> *node)
>  		return strdup("<NULL>");
>  
>  	for (n = node; n; n = n->parent) {
> -		len += strlen(n->dn_name);
> +		len += strlen(pdbg_target_to_priv(n)->dn_name);
>  		if (n->parent || n == node)
>  			len++;
>  	}
> @@ -227,9 +238,9 @@ static char *dt_get_path(const struct pdbg_target
> *node)
>  	assert(path);
>  	p = path + len;
>  	for (n = node; n; n = n->parent) {
> -		len = strlen(n->dn_name);
> +		len = strlen(pdbg_target_to_priv(n)->dn_name);
>  		p -= len;
> -		memcpy(p, n->dn_name, len);
> +		memcpy(p, pdbg_target_to_priv(n)->dn_name, len);
>  		if (n->parent || n == node)
>  			*(--p) = '/';
>  	}
> @@ -272,7 +283,8 @@ static const char *__dt_path_split(const char *p,
>  
>  static struct pdbg_target *dt_find_by_path(struct pdbg_target *root,
> const char *path)
>  {
> -	struct pdbg_target *n;
> +	struct pdbg_target_priv *n;
> +	struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root);
>  	const char *pn, *pa = NULL, *p = path, *nn = NULL, *na = NULL;
>  	unsigned int pnl, pal, nnl, nal;
>  	bool match;
> @@ -286,7 +298,7 @@ static struct pdbg_target *dt_find_by_path(struct
> pdbg_target *root, const char
>  
>  		/* Compare with each child node */
>  		match = false;
> -		list_for_each(&root->children, n, list) {
> +		list_for_each(&root_priv->children, n, list) {
>  			match = true;
>  			__dt_path_split(n->dn_name, &nn, &nnl, &na,
> &nal);
>  			if (pnl && (pnl != nnl || strncmp(pn, nn,
> pnl)))
> @@ -294,7 +306,7 @@ static struct pdbg_target *dt_find_by_path(struct
> pdbg_target *root, const char
>  			if (pal && (pal != nal || strncmp(pa, na,
> pal)))
>  				match = false;
>  			if (match) {
> -				root = n;
> +				root = &n->target;
>  				break;
>  			}
>  		}
> @@ -311,7 +323,7 @@ static struct dt_property *dt_find_property(const
> struct pdbg_target *node,
>  {
>  	struct dt_property *i = NULL;
>  
> -	list_for_each(&node->properties, i, list)
> +	list_for_each(&pdbg_target_to_priv(node)->properties, i, list)
>  		if (strcmp(i->name, name) == 0)
>  			return i;
>  	return NULL;
> @@ -341,7 +353,7 @@ static struct dt_property *new_property(struct
> pdbg_target *node,
>  
>  	p->name = take_name(name);
>  	p->len = size;
> -	list_add_tail(&node->properties, &p->list);
> +	list_add_tail(&pdbg_target_to_priv(node)->properties, &p-
> >list);
>  	return p;
>  }
>  
> @@ -349,6 +361,7 @@ static struct dt_property *dt_add_property(struct
> pdbg_target *node,
>  				    const char *name,
>  				    const void *val, size_t size)
>  {
> +	struct pdbg_target_priv *node_priv = pdbg_target_to_priv(node);
>  	struct dt_property *p;
>  
>  	/*
> @@ -358,9 +371,9 @@ static struct dt_property *dt_add_property(struct
> pdbg_target *node,
>  	if (strcmp(name, "linux,phandle") == 0 ||
>  	    strcmp(name, "phandle") == 0) {
>  		assert(size == 4);
> -		node->phandle = *(const u32 *)val;
> -		if (node->phandle >= last_phandle)
> -			last_phandle = node->phandle;
> +		node_priv->phandle = *(const u32 *)val;
> +		if (node_priv->phandle >= last_phandle)
> +			last_phandle = node_priv->phandle;
>  		return NULL;
>  	}
>  
> @@ -422,21 +435,23 @@ static u32 dt_property_get_cell(const struct
> dt_property *prop, u32 index)
>  /* First child of this node. */
>  static struct pdbg_target *dt_first(const struct pdbg_target *root)
>  {
> -	return list_top(&root->children, struct pdbg_target, list);
> +	return &list_top(&pdbg_target_to_priv(root)->children, struct
> pdbg_target_priv, list)->target;
>  }
>  
>  /* Return next node, or NULL. */
>  static struct pdbg_target *dt_next(const struct pdbg_target *root,
>  			const struct pdbg_target *prev)
>  {
> +	struct pdbg_target_priv *prev_priv = pdbg_target_to_priv(prev);
> +
>  	/* Children? */
> -	if (!list_empty(&prev->children))
> +	if (!list_empty(&prev_priv->children))
>  		return dt_first(prev);
>  
>  	do {
>  		/* More siblings? */
> -		if (prev->list.next != &prev->parent->children.n)
> -			return list_entry(prev->list.next, struct
> pdbg_target,list);
> +		if (prev_priv->list.next != &pdbg_target_to_priv(prev-
> >parent)->children.n)
> +			return &list_entry(prev_priv->list.next, struct
> pdbg_target_priv, list)->target;
>  
>  		/* No more siblings, move up to parent. */
>  		prev = prev->parent;
> @@ -563,11 +578,11 @@ static int dt_expand_node(struct pdbg_target
> *node, const void *fdt, int fdt_nod
>  			name = fdt_string(fdt, fdt32_to_cpu(prop-
> >nameoff));
>  			if (strcmp("index", name) == 0) {
>  				memcpy(&data, prop->data,
> sizeof(data));
> -				node->index = fdt32_to_cpu(data);
> +				pdbg_target_to_priv(node)->index =
> fdt32_to_cpu(data);
>  			}
>  
>  			if (strcmp("status", name) == 0)
> -				node->status = str_to_status(prop-
> >data);
> +				pdbg_target_to_priv(node)->status =
> str_to_status(prop->data);
>  
>  			dt_add_property(node, name, prop->data,
>  					fdt32_to_cpu(prop->len));
> diff --git a/libpdbg/hwunit.c b/libpdbg/hwunit.c
> index c7ec63d..5c40d23 100644
> --- a/libpdbg/hwunit.c
> +++ b/libpdbg/hwunit.c
> @@ -21,6 +21,9 @@
>  
>  #define MAX_HW_UNITS	1024
>  
> +struct pdbg_target pdbg_target_size;
> +uint8_t pdbg_target_size_test[sizeof(struct pdbg_target)];
> +
>  static const struct hw_unit_info *g_hw_unit[MAX_HW_UNITS];
>  static int g_hw_unit_count;
>  
> diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c
> index 0c5a451..bec2488 100644
> --- a/libpdbg/libpdbg.c
> +++ b/libpdbg/libpdbg.c
> @@ -9,6 +9,7 @@ static pdbg_progress_tick_t progress_tick;
>  struct pdbg_target *__pdbg_next_target(const char *class, struct
> pdbg_target *parent, struct pdbg_target *last)
>  {
>  	struct pdbg_target *next, *tmp;
> +	struct pdbg_target_priv *last_priv;
>  	struct pdbg_target_class *target_class;
>  
>  	if (class && !find_target_class(class))
> @@ -17,15 +18,17 @@ struct pdbg_target *__pdbg_next_target(const char
> *class, struct pdbg_target *pa
>  	target_class = require_target_class(class);
>  
>  retry:
> +	last_priv = pdbg_target_to_priv(last);
> +
>  	/* No more targets left to check in this class */
> -	if ((last && last->class_link.next == &target_class->targets.n) 
> ||
> +	if ((last && last_priv->class_link.next == &target_class-
> >targets.n) ||
>  	    list_empty(&target_class->targets))
>  		return NULL;
>  
>  	if (last)
> -		next = list_entry(last->class_link.next, struct
> pdbg_target, class_link);
> +		next = &list_entry(last_priv->class_link.next, struct
> pdbg_target_priv, class_link)->target;
>  	else
> -		if (!(next = list_top(&target_class->targets, struct
> pdbg_target, class_link)))
> +		if (!(next = &list_top(&target_class->targets, struct
> pdbg_target_priv, class_link)->target))
>  			return NULL;
>  
>  	if (!parent)
> @@ -45,21 +48,24 @@ retry:
>  
>  struct pdbg_target *__pdbg_next_child_target(struct pdbg_target
> *parent, struct pdbg_target *last)
>  {
> -	if (!parent || list_empty(&parent->children))
> +	struct pdbg_target_priv *parent_priv =
> pdbg_target_to_priv(parent);
> +	struct pdbg_target_priv *last_priv = pdbg_target_to_priv(last);
> +
> +	if (!parent || list_empty(&parent_priv->children))
>  		return NULL;
>  
>  	if (!last)
> -		return list_top(&parent->children, struct pdbg_target,
> list);
> +		return &list_top(&parent_priv->children, struct
> pdbg_target_priv, list)->target;
>  
> -	if (last->list.next == &parent->children.n)
> +	if (last_priv->list.next == &parent_priv->children.n)
>  		return NULL;
>  
> -	return list_entry(last->list.next, struct pdbg_target, list);
> +	return &list_entry(last_priv->list.next, struct
> pdbg_target_priv, list)->target;
>  }
>  
>  enum pdbg_target_status pdbg_target_status(struct pdbg_target
> *target)
>  {
> -	return target->status;
> +	return pdbg_target_to_priv(target)->status;
>  }
>  
>  void pdbg_target_status_set(struct pdbg_target *target, enum
> pdbg_target_status status)
> @@ -68,7 +74,7 @@ void pdbg_target_status_set(struct pdbg_target
> *target, enum pdbg_target_status
>  	 * blow up obviously if this happens */
>  	assert(status == PDBG_TARGET_DISABLED || status ==
> PDBG_TARGET_MUSTEXIST);
>  
> -	target->status = status;
> +	pdbg_target_to_priv(target)->status = status;
>  }
>  
>  /* Searches up the tree and returns the first valid index found */
> @@ -76,12 +82,12 @@ uint32_t pdbg_target_index(struct pdbg_target
> *target)
>  {
>  	struct pdbg_target *dn;
>  
> -	for (dn = target; dn && dn->index == -1; dn = dn->parent);
> +	for (dn = target; dn && pdbg_target_to_priv(dn)->index == -1;
> dn = dn->parent);
>  
>  	if (!dn)
>  		return -1;
>  	else
> -		return dn->index;
> +		return pdbg_target_to_priv(dn)->index;
>  }
>  
>  /* Find a target parent from the given class */
> @@ -132,7 +138,7 @@ char *pdbg_target_name(struct pdbg_target
> *target)
>  
>  const char *pdbg_target_dn_name(struct pdbg_target *target)
>  {
> -	return target->dn_name;
> +	return pdbg_target_to_priv(target)->dn_name;
>  }
>  
>  int pdbg_target_u32_property(struct pdbg_target *target, const char
> *name, uint32_t *val)
> diff --git a/libpdbg/target.c b/libpdbg/target.c
> index 5808d02..ce2bb2d 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -400,7 +400,7 @@ enum pdbg_target_status pdbg_target_probe(struct
> pdbg_target *target)
>  		case PDBG_TARGET_NONEXISTENT:
>  			/* The parent doesn't exist neither does it's
>  			 * children */
> -			target->status = PDBG_TARGET_NONEXISTENT;
> +			pdbg_target_to_priv(target)->status =
> PDBG_TARGET_NONEXISTENT;
>  			return PDBG_TARGET_NONEXISTENT;
>  
>  		case PDBG_TARGET_DISABLED:
> @@ -427,11 +427,11 @@ enum pdbg_target_status
> pdbg_target_probe(struct pdbg_target *target)
>  	if (target->probe && target->probe(target)) {
>  		/* Could not find the target */
>  		assert(pdbg_target_status(target) !=
> PDBG_TARGET_MUSTEXIST);
> -		target->status = PDBG_TARGET_NONEXISTENT;
> +		pdbg_target_to_priv(target)->status =
> PDBG_TARGET_NONEXISTENT;
>  		return PDBG_TARGET_NONEXISTENT;
>  	}
>  
> -	target->status = PDBG_TARGET_ENABLED;
> +	pdbg_target_to_priv(target)->status = PDBG_TARGET_ENABLED;
>  	return PDBG_TARGET_ENABLED;
>  }
>  
> @@ -449,7 +449,7 @@ void pdbg_target_release(struct pdbg_target
> *target)
>  	/* Release the target */
>  	if (target->release)
>  		target->release(target);
> -	target->status = PDBG_TARGET_RELEASED;
> +	pdbg_target_to_priv(target)->status = PDBG_TARGET_RELEASED;
>  }
>  
>  /*
> @@ -477,10 +477,10 @@ bool pdbg_target_is_class(struct pdbg_target
> *target, const char *class)
>  
>  void *pdbg_target_priv(struct pdbg_target *target)
>  {
> -	return target->priv;
> +	return pdbg_target_to_priv(target)->priv;
>  }
>  
>  void pdbg_target_priv_set(struct pdbg_target *target, void *priv)
>  {
> -	target->priv = priv;
> +	pdbg_target_to_priv(target)->priv = priv;
>  }
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index 2ecbf98..ba2b20d 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -39,19 +39,25 @@ struct pdbg_target {
>  	int (*probe)(struct pdbg_target *target);
>  	void (*release)(struct pdbg_target *target);
>  	uint64_t (*translate)(struct pdbg_target *target, uint64_t
> addr);
> +	struct pdbg_target *parent;
> +};
> +
> +struct pdbg_target_priv {
>  	int index;
>  	enum pdbg_target_status status;
>  	const char *dn_name;
>  	struct list_node list;
>  	struct list_head properties;
>  	struct list_head children;
> -	struct pdbg_target *parent;
>  	u32 phandle;
>  	bool probed;
>  	struct list_node class_link;
> +	struct pdbg_target target;
>  	void *priv;
>  };
>  
> +#define pdbg_target_to_priv(x) (x ? container_of(x, struct
> pdbg_target_priv, target) : NULL)
> +
>  struct pdbg_target *require_target_parent(struct pdbg_target
> *target);
>  struct pdbg_target_class *find_target_class(const char *name);
>  struct pdbg_target_class *require_target_class(const char *name);
> -- 
> 2.20.1
> 

Amitay.
Alistair Popple Sept. 4, 2019, 3:50 a.m. UTC | #2
Amitay,

> Even though internally libpdbg uses pdbg_target to store part of
> hardware unit, exposing pdbg_target as a hardware unit is a bad idea. 
> From libpdbg api it's clear that pdbg_target represents an instance of
> a hardware unit.  So overloading pdbg_target as a hardware unit adds
> more confusion.
>
> It would make more sense (to me at least) if we call new structure
> pdbg_hwunit (or similar but different from pdbg_target).  This will
> make the libpdbg api cleaner and less confusing.  (What we really need
> is virtual base class and every hardware unit is derived from that. 
> Need a good way to do that in C.)

Ok, it took me a moment to work out what the issue was and what you were 
suggesting but now that I've figured it out I agree - it would be much clearer/
cleaner to differentiate between the two. Will do a v2 that fixes it.

Next up, converting to C++ (-:

> Also, let's not add any structures to libpdbg API without pdbg_ prefix
> (e.g. hw_unit_info).  I guess this will involve a bit of churn through
> libpdbg, but in the long run it will be better.

Thanks, that is also my intent but I missed this one. As they all (should) 
only get created by a macro I doubt it will involve too much churn.

- Alistair

> Amitay. 
>  
> > 
> > Signed-off-by: Alistair Popple <alistair@popple.id.au>
> > ---
> >  libpdbg/device.c  | 95 +++++++++++++++++++++++++++----------------
> > ----
> >  libpdbg/hwunit.c  |  3 ++
> >  libpdbg/libpdbg.c | 30 +++++++++------
> >  libpdbg/target.c  | 12 +++---
> >  libpdbg/target.h  |  8 +++-
> >  5 files changed, 89 insertions(+), 59 deletions(-)
> > 
> > diff --git a/libpdbg/device.c b/libpdbg/device.c
> > index efa9ce4..e009a1a 100644
> > --- a/libpdbg/device.c
> > +++ b/libpdbg/device.c
> > @@ -67,6 +67,7 @@ static const char *take_name(const char *name)
> >  /* Adds information representing an actual target */
> >  static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int
> > node_offset)
> >  {
> > +	struct pdbg_target_priv *target_priv;
> >  	struct pdbg_target *target;
> >  	struct pdbg_target_class *target_class;
> >  	const struct hw_unit_info *hw_info = NULL;
> > @@ -98,19 +99,21 @@ static struct pdbg_target
> > *dt_pdbg_target_new(const void *fdt, int node_offset)
> >  		/* Couldn't find anything implementing this target */
> >  		return NULL;
> >  
> > -	target = calloc(1, size);
> > -	if (!target) {
> > +	target_priv = calloc(1, sizeof(struct pdbg_target_priv) -
> > sizeof(struct pdbg_target) + size);
> > +	if (!target_priv) {
> >  		prerror("Failed to allocate node\n");
> >  		abort();
> >  	}
> >  
> > +	target = &target_priv->target;
> > +
> >  	/* hw_info->hw_unit points to a per-target struct type. This
> >  	 * works because the first member in the per-target struct is
> >  	 * guaranteed to be the struct pdbg_target (see the comment
> >  	 * above DECLARE_HW_UNIT). */
> >  	memcpy(target, hw_info->hw_unit, size);
> >  	target_class = get_target_class(target);
> > -	list_add_tail(&target_class->targets, &target->class_link);
> > +	list_add_tail(&target_class->targets, &target_priv-
> > >class_link);
> >  
> >  	return target;
> >  }
> > @@ -118,31 +121,36 @@ static struct pdbg_target
> > *dt_pdbg_target_new(const void *fdt, int node_offset)
> >  static struct pdbg_target *dt_new_node(const char *name, const void
> > *fdt, int node_offset)
> >  {
> >  	struct pdbg_target *node = NULL;
> > -	size_t size = sizeof(*node);
> > +	struct pdbg_target_priv *target_priv;
> >  
> > -	if (fdt)
> > +	if (fdt) {
> >  		node = dt_pdbg_target_new(fdt, node_offset);
> > -
> > -	if (!node)
> > -		node = calloc(1, size);
> > +		target_priv = pdbg_target_to_priv(node);
> > +	}
> >  
> >  	if (!node) {
> > +		target_priv = calloc(1, sizeof(*target_priv));
> > +		node = &target_priv->target;
> > +	}
> > +
> > +	if (!node && !target_priv) {
> >  		prerror("Failed to allocate node\n");
> >  		abort();
> >  	}
> >  
> > -	node->dn_name = take_name(name);
> > +	target_priv->dn_name = take_name(name);
> >  	node->parent = NULL;
> > -	list_head_init(&node->properties);
> > -	list_head_init(&node->children);
> > -	node->phandle = ++last_phandle;
> > +	list_head_init(&target_priv->properties);
> > +	list_head_init(&target_priv->children);
> > +	target_priv->phandle = ++last_phandle;
> >  
> >  	return node;
> >  }
> >  
> >  static const char *get_unitname(const struct pdbg_target *node)
> >  {
> > -	const char *c = strchr(node->dn_name, '@');
> > +	struct pdbg_target_priv *target_priv =
> > pdbg_target_to_priv(node);
> > +	const char *c = strchr(target_priv->dn_name, '@');
> >  
> >  	if (!c)
> >  		return NULL;
> > @@ -152,13 +160,14 @@ static const char *get_unitname(const struct
> > pdbg_target *node)
> >  
> >  static int dt_cmp_subnodes(const struct pdbg_target *a, const struct
> > pdbg_target *b)
> >  {
> > +	struct pdbg_target_priv *priv_a = pdbg_target_to_priv(a),
> > *priv_b = pdbg_target_to_priv(b);
> >  	const char *a_unit = get_unitname(a);
> >  	const char *b_unit = get_unitname(b);
> >  
> > -	ptrdiff_t basenamelen = a_unit - a->dn_name;
> > +	ptrdiff_t basenamelen = a_unit - priv_a->dn_name;
> >  
> >  	/* sort hex unit addresses by number */
> > -	if (a_unit && b_unit && !strncmp(a->dn_name, b->dn_name,
> > basenamelen)) {
> > +	if (a_unit && b_unit && !strncmp(priv_a->dn_name, priv_b-
> > >dn_name, basenamelen)) {
> >  		unsigned long long a_num, b_num;
> >  		char *a_end, *b_end;
> >  
> > @@ -170,29 +179,31 @@ static int dt_cmp_subnodes(const struct
> > pdbg_target *a, const struct pdbg_target
> >  			return (a_num > b_num) - (a_num < b_num);
> >  	}
> >  
> > -	return strcmp(a->dn_name, b->dn_name);
> > +	return strcmp(priv_a->dn_name, priv_b->dn_name);
> >  }
> >  
> >  static bool dt_attach_root(struct pdbg_target *parent, struct
> > pdbg_target *root)
> >  {
> > -	struct pdbg_target *node = NULL;
> > +	struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root);
> > +	struct pdbg_target_priv *parent_priv =
> > pdbg_target_to_priv(parent);
> > +	struct pdbg_target_priv *node = NULL;
> >  
> >  	assert(!root->parent);
> >  
> > -	if (list_empty(&parent->children)) {
> > -		list_add(&parent->children, &root->list);
> > +	if (list_empty(&parent_priv->children)) {
> > +		list_add(&parent_priv->children, &root_priv->list);
> >  		root->parent = parent;
> >  
> >  		return true;
> >  	}
> >  
> > -	dt_for_each_child(parent, node) {
> > -		int cmp = dt_cmp_subnodes(node, root);
> > +	dt_for_each_child(parent_priv, node) {
> > +		int cmp = dt_cmp_subnodes(&node->target, root);
> >  
> >  		/* Look for duplicates */
> >  		if (cmp == 0) {
> >  			prerror("DT: %s failed, duplicate %s\n",
> > -				__func__, root->dn_name);
> > +				__func__, root_priv->dn_name);
> >  			return false;
> >  		}
> >  
> > @@ -202,7 +213,7 @@ static bool dt_attach_root(struct pdbg_target
> > *parent, struct pdbg_target *root)
> >  			break;
> >  	}
> >  
> > -	list_add_before(&parent->children, &root->list, &node->list);
> > +	list_add_before(&parent_priv->children, &root_priv->list,
> > &node->list);
> >  	root->parent = parent;
> >  
> >  	return true;
> > @@ -219,7 +230,7 @@ static char *dt_get_path(const struct pdbg_target
> > *node)
> >  		return strdup("<NULL>");
> >  
> >  	for (n = node; n; n = n->parent) {
> > -		len += strlen(n->dn_name);
> > +		len += strlen(pdbg_target_to_priv(n)->dn_name);
> >  		if (n->parent || n == node)
> >  			len++;
> >  	}
> > @@ -227,9 +238,9 @@ static char *dt_get_path(const struct pdbg_target
> > *node)
> >  	assert(path);
> >  	p = path + len;
> >  	for (n = node; n; n = n->parent) {
> > -		len = strlen(n->dn_name);
> > +		len = strlen(pdbg_target_to_priv(n)->dn_name);
> >  		p -= len;
> > -		memcpy(p, n->dn_name, len);
> > +		memcpy(p, pdbg_target_to_priv(n)->dn_name, len);
> >  		if (n->parent || n == node)
> >  			*(--p) = '/';
> >  	}
> > @@ -272,7 +283,8 @@ static const char *__dt_path_split(const char *p,
> >  
> >  static struct pdbg_target *dt_find_by_path(struct pdbg_target *root,
> > const char *path)
> >  {
> > -	struct pdbg_target *n;
> > +	struct pdbg_target_priv *n;
> > +	struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root);
> >  	const char *pn, *pa = NULL, *p = path, *nn = NULL, *na = NULL;
> >  	unsigned int pnl, pal, nnl, nal;
> >  	bool match;
> > @@ -286,7 +298,7 @@ static struct pdbg_target *dt_find_by_path(struct
> > pdbg_target *root, const char
> >  
> >  		/* Compare with each child node */
> >  		match = false;
> > -		list_for_each(&root->children, n, list) {
> > +		list_for_each(&root_priv->children, n, list) {
> >  			match = true;
> >  			__dt_path_split(n->dn_name, &nn, &nnl, &na,
> > &nal);
> >  			if (pnl && (pnl != nnl || strncmp(pn, nn,
> > pnl)))
> > @@ -294,7 +306,7 @@ static struct pdbg_target *dt_find_by_path(struct
> > pdbg_target *root, const char
> >  			if (pal && (pal != nal || strncmp(pa, na,
> > pal)))
> >  				match = false;
> >  			if (match) {
> > -				root = n;
> > +				root = &n->target;
> >  				break;
> >  			}
> >  		}
> > @@ -311,7 +323,7 @@ static struct dt_property *dt_find_property(const
> > struct pdbg_target *node,
> >  {
> >  	struct dt_property *i = NULL;
> >  
> > -	list_for_each(&node->properties, i, list)
> > +	list_for_each(&pdbg_target_to_priv(node)->properties, i, list)
> >  		if (strcmp(i->name, name) == 0)
> >  			return i;
> >  	return NULL;
> > @@ -341,7 +353,7 @@ static struct dt_property *new_property(struct
> > pdbg_target *node,
> >  
> >  	p->name = take_name(name);
> >  	p->len = size;
> > -	list_add_tail(&node->properties, &p->list);
> > +	list_add_tail(&pdbg_target_to_priv(node)->properties, &p-
> > >list);
> >  	return p;
> >  }
> >  
> > @@ -349,6 +361,7 @@ static struct dt_property *dt_add_property(struct
> > pdbg_target *node,
> >  				    const char *name,
> >  				    const void *val, size_t size)
> >  {
> > +	struct pdbg_target_priv *node_priv = pdbg_target_to_priv(node);
> >  	struct dt_property *p;
> >  
> >  	/*
> > @@ -358,9 +371,9 @@ static struct dt_property *dt_add_property(struct
> > pdbg_target *node,
> >  	if (strcmp(name, "linux,phandle") == 0 ||
> >  	    strcmp(name, "phandle") == 0) {
> >  		assert(size == 4);
> > -		node->phandle = *(const u32 *)val;
> > -		if (node->phandle >= last_phandle)
> > -			last_phandle = node->phandle;
> > +		node_priv->phandle = *(const u32 *)val;
> > +		if (node_priv->phandle >= last_phandle)
> > +			last_phandle = node_priv->phandle;
> >  		return NULL;
> >  	}
> >  
> > @@ -422,21 +435,23 @@ static u32 dt_property_get_cell(const struct
> > dt_property *prop, u32 index)
> >  /* First child of this node. */
> >  static struct pdbg_target *dt_first(const struct pdbg_target *root)
> >  {
> > -	return list_top(&root->children, struct pdbg_target, list);
> > +	return &list_top(&pdbg_target_to_priv(root)->children, struct
> > pdbg_target_priv, list)->target;
> >  }
> >  
> >  /* Return next node, or NULL. */
> >  static struct pdbg_target *dt_next(const struct pdbg_target *root,
> >  			const struct pdbg_target *prev)
> >  {
> > +	struct pdbg_target_priv *prev_priv = pdbg_target_to_priv(prev);
> > +
> >  	/* Children? */
> > -	if (!list_empty(&prev->children))
> > +	if (!list_empty(&prev_priv->children))
> >  		return dt_first(prev);
> >  
> >  	do {
> >  		/* More siblings? */
> > -		if (prev->list.next != &prev->parent->children.n)
> > -			return list_entry(prev->list.next, struct
> > pdbg_target,list);
> > +		if (prev_priv->list.next != &pdbg_target_to_priv(prev-
> > >parent)->children.n)
> > +			return &list_entry(prev_priv->list.next, struct
> > pdbg_target_priv, list)->target;
> >  
> >  		/* No more siblings, move up to parent. */
> >  		prev = prev->parent;
> > @@ -563,11 +578,11 @@ static int dt_expand_node(struct pdbg_target
> > *node, const void *fdt, int fdt_nod
> >  			name = fdt_string(fdt, fdt32_to_cpu(prop-
> > >nameoff));
> >  			if (strcmp("index", name) == 0) {
> >  				memcpy(&data, prop->data,
> > sizeof(data));
> > -				node->index = fdt32_to_cpu(data);
> > +				pdbg_target_to_priv(node)->index =
> > fdt32_to_cpu(data);
> >  			}
> >  
> >  			if (strcmp("status", name) == 0)
> > -				node->status = str_to_status(prop-
> > >data);
> > +				pdbg_target_to_priv(node)->status =
> > str_to_status(prop->data);
> >  
> >  			dt_add_property(node, name, prop->data,
> >  					fdt32_to_cpu(prop->len));
> > diff --git a/libpdbg/hwunit.c b/libpdbg/hwunit.c
> > index c7ec63d..5c40d23 100644
> > --- a/libpdbg/hwunit.c
> > +++ b/libpdbg/hwunit.c
> > @@ -21,6 +21,9 @@
> >  
> >  #define MAX_HW_UNITS	1024
> >  
> > +struct pdbg_target pdbg_target_size;
> > +uint8_t pdbg_target_size_test[sizeof(struct pdbg_target)];
> > +
> >  static const struct hw_unit_info *g_hw_unit[MAX_HW_UNITS];
> >  static int g_hw_unit_count;
> >  
> > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c
> > index 0c5a451..bec2488 100644
> > --- a/libpdbg/libpdbg.c
> > +++ b/libpdbg/libpdbg.c
> > @@ -9,6 +9,7 @@ static pdbg_progress_tick_t progress_tick;
> >  struct pdbg_target *__pdbg_next_target(const char *class, struct
> > pdbg_target *parent, struct pdbg_target *last)
> >  {
> >  	struct pdbg_target *next, *tmp;
> > +	struct pdbg_target_priv *last_priv;
> >  	struct pdbg_target_class *target_class;
> >  
> >  	if (class && !find_target_class(class))
> > @@ -17,15 +18,17 @@ struct pdbg_target *__pdbg_next_target(const char
> > *class, struct pdbg_target *pa
> >  	target_class = require_target_class(class);
> >  
> >  retry:
> > +	last_priv = pdbg_target_to_priv(last);
> > +
> >  	/* No more targets left to check in this class */
> > -	if ((last && last->class_link.next == &target_class->targets.n) 
> > ||
> > +	if ((last && last_priv->class_link.next == &target_class-
> > >targets.n) ||
> >  	    list_empty(&target_class->targets))
> >  		return NULL;
> >  
> >  	if (last)
> > -		next = list_entry(last->class_link.next, struct
> > pdbg_target, class_link);
> > +		next = &list_entry(last_priv->class_link.next, struct
> > pdbg_target_priv, class_link)->target;
> >  	else
> > -		if (!(next = list_top(&target_class->targets, struct
> > pdbg_target, class_link)))
> > +		if (!(next = &list_top(&target_class->targets, struct
> > pdbg_target_priv, class_link)->target))
> >  			return NULL;
> >  
> >  	if (!parent)
> > @@ -45,21 +48,24 @@ retry:
> >  
> >  struct pdbg_target *__pdbg_next_child_target(struct pdbg_target
> > *parent, struct pdbg_target *last)
> >  {
> > -	if (!parent || list_empty(&parent->children))
> > +	struct pdbg_target_priv *parent_priv =
> > pdbg_target_to_priv(parent);
> > +	struct pdbg_target_priv *last_priv = pdbg_target_to_priv(last);
> > +
> > +	if (!parent || list_empty(&parent_priv->children))
> >  		return NULL;
> >  
> >  	if (!last)
> > -		return list_top(&parent->children, struct pdbg_target,
> > list);
> > +		return &list_top(&parent_priv->children, struct
> > pdbg_target_priv, list)->target;
> >  
> > -	if (last->list.next == &parent->children.n)
> > +	if (last_priv->list.next == &parent_priv->children.n)
> >  		return NULL;
> >  
> > -	return list_entry(last->list.next, struct pdbg_target, list);
> > +	return &list_entry(last_priv->list.next, struct
> > pdbg_target_priv, list)->target;
> >  }
> >  
> >  enum pdbg_target_status pdbg_target_status(struct pdbg_target
> > *target)
> >  {
> > -	return target->status;
> > +	return pdbg_target_to_priv(target)->status;
> >  }
> >  
> >  void pdbg_target_status_set(struct pdbg_target *target, enum
> > pdbg_target_status status)
> > @@ -68,7 +74,7 @@ void pdbg_target_status_set(struct pdbg_target
> > *target, enum pdbg_target_status
> >  	 * blow up obviously if this happens */
> >  	assert(status == PDBG_TARGET_DISABLED || status ==
> > PDBG_TARGET_MUSTEXIST);
> >  
> > -	target->status = status;
> > +	pdbg_target_to_priv(target)->status = status;
> >  }
> >  
> >  /* Searches up the tree and returns the first valid index found */
> > @@ -76,12 +82,12 @@ uint32_t pdbg_target_index(struct pdbg_target
> > *target)
> >  {
> >  	struct pdbg_target *dn;
> >  
> > -	for (dn = target; dn && dn->index == -1; dn = dn->parent);
> > +	for (dn = target; dn && pdbg_target_to_priv(dn)->index == -1;
> > dn = dn->parent);
> >  
> >  	if (!dn)
> >  		return -1;
> >  	else
> > -		return dn->index;
> > +		return pdbg_target_to_priv(dn)->index;
> >  }
> >  
> >  /* Find a target parent from the given class */
> > @@ -132,7 +138,7 @@ char *pdbg_target_name(struct pdbg_target
> > *target)
> >  
> >  const char *pdbg_target_dn_name(struct pdbg_target *target)
> >  {
> > -	return target->dn_name;
> > +	return pdbg_target_to_priv(target)->dn_name;
> >  }
> >  
> >  int pdbg_target_u32_property(struct pdbg_target *target, const char
> > *name, uint32_t *val)
> > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > index 5808d02..ce2bb2d 100644
> > --- a/libpdbg/target.c
> > +++ b/libpdbg/target.c
> > @@ -400,7 +400,7 @@ enum pdbg_target_status pdbg_target_probe(struct
> > pdbg_target *target)
> >  		case PDBG_TARGET_NONEXISTENT:
> >  			/* The parent doesn't exist neither does it's
> >  			 * children */
> > -			target->status = PDBG_TARGET_NONEXISTENT;
> > +			pdbg_target_to_priv(target)->status =
> > PDBG_TARGET_NONEXISTENT;
> >  			return PDBG_TARGET_NONEXISTENT;
> >  
> >  		case PDBG_TARGET_DISABLED:
> > @@ -427,11 +427,11 @@ enum pdbg_target_status
> > pdbg_target_probe(struct pdbg_target *target)
> >  	if (target->probe && target->probe(target)) {
> >  		/* Could not find the target */
> >  		assert(pdbg_target_status(target) !=
> > PDBG_TARGET_MUSTEXIST);
> > -		target->status = PDBG_TARGET_NONEXISTENT;
> > +		pdbg_target_to_priv(target)->status =
> > PDBG_TARGET_NONEXISTENT;
> >  		return PDBG_TARGET_NONEXISTENT;
> >  	}
> >  
> > -	target->status = PDBG_TARGET_ENABLED;
> > +	pdbg_target_to_priv(target)->status = PDBG_TARGET_ENABLED;
> >  	return PDBG_TARGET_ENABLED;
> >  }
> >  
> > @@ -449,7 +449,7 @@ void pdbg_target_release(struct pdbg_target
> > *target)
> >  	/* Release the target */
> >  	if (target->release)
> >  		target->release(target);
> > -	target->status = PDBG_TARGET_RELEASED;
> > +	pdbg_target_to_priv(target)->status = PDBG_TARGET_RELEASED;
> >  }
> >  
> >  /*
> > @@ -477,10 +477,10 @@ bool pdbg_target_is_class(struct pdbg_target
> > *target, const char *class)
> >  
> >  void *pdbg_target_priv(struct pdbg_target *target)
> >  {
> > -	return target->priv;
> > +	return pdbg_target_to_priv(target)->priv;
> >  }
> >  
> >  void pdbg_target_priv_set(struct pdbg_target *target, void *priv)
> >  {
> > -	target->priv = priv;
> > +	pdbg_target_to_priv(target)->priv = priv;
> >  }
> > diff --git a/libpdbg/target.h b/libpdbg/target.h
> > index 2ecbf98..ba2b20d 100644
> > --- a/libpdbg/target.h
> > +++ b/libpdbg/target.h
> > @@ -39,19 +39,25 @@ struct pdbg_target {
> >  	int (*probe)(struct pdbg_target *target);
> >  	void (*release)(struct pdbg_target *target);
> >  	uint64_t (*translate)(struct pdbg_target *target, uint64_t
> > addr);
> > +	struct pdbg_target *parent;
> > +};
> > +
> > +struct pdbg_target_priv {
> >  	int index;
> >  	enum pdbg_target_status status;
> >  	const char *dn_name;
> >  	struct list_node list;
> >  	struct list_head properties;
> >  	struct list_head children;
> > -	struct pdbg_target *parent;
> >  	u32 phandle;
> >  	bool probed;
> >  	struct list_node class_link;
> > +	struct pdbg_target target;
> >  	void *priv;
> >  };
> >  
> > +#define pdbg_target_to_priv(x) (x ? container_of(x, struct
> > pdbg_target_priv, target) : NULL)
> > +
> >  struct pdbg_target *require_target_parent(struct pdbg_target
> > *target);
> >  struct pdbg_target_class *find_target_class(const char *name);
> >  struct pdbg_target_class *require_target_class(const char *name);
> 
> Amitay.
>

Patch
diff mbox series

diff --git a/libpdbg/device.c b/libpdbg/device.c
index efa9ce4..e009a1a 100644
--- a/libpdbg/device.c
+++ b/libpdbg/device.c
@@ -67,6 +67,7 @@  static const char *take_name(const char *name)
 /* Adds information representing an actual target */
 static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset)
 {
+	struct pdbg_target_priv *target_priv;
 	struct pdbg_target *target;
 	struct pdbg_target_class *target_class;
 	const struct hw_unit_info *hw_info = NULL;
@@ -98,19 +99,21 @@  static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset)
 		/* Couldn't find anything implementing this target */
 		return NULL;
 
-	target = calloc(1, size);
-	if (!target) {
+	target_priv = calloc(1, sizeof(struct pdbg_target_priv) - sizeof(struct pdbg_target) + size);
+	if (!target_priv) {
 		prerror("Failed to allocate node\n");
 		abort();
 	}
 
+	target = &target_priv->target;
+
 	/* hw_info->hw_unit points to a per-target struct type. This
 	 * works because the first member in the per-target struct is
 	 * guaranteed to be the struct pdbg_target (see the comment
 	 * above DECLARE_HW_UNIT). */
 	memcpy(target, hw_info->hw_unit, size);
 	target_class = get_target_class(target);
-	list_add_tail(&target_class->targets, &target->class_link);
+	list_add_tail(&target_class->targets, &target_priv->class_link);
 
 	return target;
 }
@@ -118,31 +121,36 @@  static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset)
 static struct pdbg_target *dt_new_node(const char *name, const void *fdt, int node_offset)
 {
 	struct pdbg_target *node = NULL;
-	size_t size = sizeof(*node);
+	struct pdbg_target_priv *target_priv;
 
-	if (fdt)
+	if (fdt) {
 		node = dt_pdbg_target_new(fdt, node_offset);
-
-	if (!node)
-		node = calloc(1, size);
+		target_priv = pdbg_target_to_priv(node);
+	}
 
 	if (!node) {
+		target_priv = calloc(1, sizeof(*target_priv));
+		node = &target_priv->target;
+	}
+
+	if (!node && !target_priv) {
 		prerror("Failed to allocate node\n");
 		abort();
 	}
 
-	node->dn_name = take_name(name);
+	target_priv->dn_name = take_name(name);
 	node->parent = NULL;
-	list_head_init(&node->properties);
-	list_head_init(&node->children);
-	node->phandle = ++last_phandle;
+	list_head_init(&target_priv->properties);
+	list_head_init(&target_priv->children);
+	target_priv->phandle = ++last_phandle;
 
 	return node;
 }
 
 static const char *get_unitname(const struct pdbg_target *node)
 {
-	const char *c = strchr(node->dn_name, '@');
+	struct pdbg_target_priv *target_priv = pdbg_target_to_priv(node);
+	const char *c = strchr(target_priv->dn_name, '@');
 
 	if (!c)
 		return NULL;
@@ -152,13 +160,14 @@  static const char *get_unitname(const struct pdbg_target *node)
 
 static int dt_cmp_subnodes(const struct pdbg_target *a, const struct pdbg_target *b)
 {
+	struct pdbg_target_priv *priv_a = pdbg_target_to_priv(a), *priv_b = pdbg_target_to_priv(b);
 	const char *a_unit = get_unitname(a);
 	const char *b_unit = get_unitname(b);
 
-	ptrdiff_t basenamelen = a_unit - a->dn_name;
+	ptrdiff_t basenamelen = a_unit - priv_a->dn_name;
 
 	/* sort hex unit addresses by number */
-	if (a_unit && b_unit && !strncmp(a->dn_name, b->dn_name, basenamelen)) {
+	if (a_unit && b_unit && !strncmp(priv_a->dn_name, priv_b->dn_name, basenamelen)) {
 		unsigned long long a_num, b_num;
 		char *a_end, *b_end;
 
@@ -170,29 +179,31 @@  static int dt_cmp_subnodes(const struct pdbg_target *a, const struct pdbg_target
 			return (a_num > b_num) - (a_num < b_num);
 	}
 
-	return strcmp(a->dn_name, b->dn_name);
+	return strcmp(priv_a->dn_name, priv_b->dn_name);
 }
 
 static bool dt_attach_root(struct pdbg_target *parent, struct pdbg_target *root)
 {
-	struct pdbg_target *node = NULL;
+	struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root);
+	struct pdbg_target_priv *parent_priv = pdbg_target_to_priv(parent);
+	struct pdbg_target_priv *node = NULL;
 
 	assert(!root->parent);
 
-	if (list_empty(&parent->children)) {
-		list_add(&parent->children, &root->list);
+	if (list_empty(&parent_priv->children)) {
+		list_add(&parent_priv->children, &root_priv->list);
 		root->parent = parent;
 
 		return true;
 	}
 
-	dt_for_each_child(parent, node) {
-		int cmp = dt_cmp_subnodes(node, root);
+	dt_for_each_child(parent_priv, node) {
+		int cmp = dt_cmp_subnodes(&node->target, root);
 
 		/* Look for duplicates */
 		if (cmp == 0) {
 			prerror("DT: %s failed, duplicate %s\n",
-				__func__, root->dn_name);
+				__func__, root_priv->dn_name);
 			return false;
 		}
 
@@ -202,7 +213,7 @@  static bool dt_attach_root(struct pdbg_target *parent, struct pdbg_target *root)
 			break;
 	}
 
-	list_add_before(&parent->children, &root->list, &node->list);
+	list_add_before(&parent_priv->children, &root_priv->list, &node->list);
 	root->parent = parent;
 
 	return true;
@@ -219,7 +230,7 @@  static char *dt_get_path(const struct pdbg_target *node)
 		return strdup("<NULL>");
 
 	for (n = node; n; n = n->parent) {
-		len += strlen(n->dn_name);
+		len += strlen(pdbg_target_to_priv(n)->dn_name);
 		if (n->parent || n == node)
 			len++;
 	}
@@ -227,9 +238,9 @@  static char *dt_get_path(const struct pdbg_target *node)
 	assert(path);
 	p = path + len;
 	for (n = node; n; n = n->parent) {
-		len = strlen(n->dn_name);
+		len = strlen(pdbg_target_to_priv(n)->dn_name);
 		p -= len;
-		memcpy(p, n->dn_name, len);
+		memcpy(p, pdbg_target_to_priv(n)->dn_name, len);
 		if (n->parent || n == node)
 			*(--p) = '/';
 	}
@@ -272,7 +283,8 @@  static const char *__dt_path_split(const char *p,
 
 static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char *path)
 {
-	struct pdbg_target *n;
+	struct pdbg_target_priv *n;
+	struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root);
 	const char *pn, *pa = NULL, *p = path, *nn = NULL, *na = NULL;
 	unsigned int pnl, pal, nnl, nal;
 	bool match;
@@ -286,7 +298,7 @@  static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char
 
 		/* Compare with each child node */
 		match = false;
-		list_for_each(&root->children, n, list) {
+		list_for_each(&root_priv->children, n, list) {
 			match = true;
 			__dt_path_split(n->dn_name, &nn, &nnl, &na, &nal);
 			if (pnl && (pnl != nnl || strncmp(pn, nn, pnl)))
@@ -294,7 +306,7 @@  static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char
 			if (pal && (pal != nal || strncmp(pa, na, pal)))
 				match = false;
 			if (match) {
-				root = n;
+				root = &n->target;
 				break;
 			}
 		}
@@ -311,7 +323,7 @@  static struct dt_property *dt_find_property(const struct pdbg_target *node,
 {
 	struct dt_property *i = NULL;
 
-	list_for_each(&node->properties, i, list)
+	list_for_each(&pdbg_target_to_priv(node)->properties, i, list)
 		if (strcmp(i->name, name) == 0)
 			return i;
 	return NULL;
@@ -341,7 +353,7 @@  static struct dt_property *new_property(struct pdbg_target *node,
 
 	p->name = take_name(name);
 	p->len = size;
-	list_add_tail(&node->properties, &p->list);
+	list_add_tail(&pdbg_target_to_priv(node)->properties, &p->list);
 	return p;
 }
 
@@ -349,6 +361,7 @@  static struct dt_property *dt_add_property(struct pdbg_target *node,
 				    const char *name,
 				    const void *val, size_t size)
 {
+	struct pdbg_target_priv *node_priv = pdbg_target_to_priv(node);
 	struct dt_property *p;
 
 	/*
@@ -358,9 +371,9 @@  static struct dt_property *dt_add_property(struct pdbg_target *node,
 	if (strcmp(name, "linux,phandle") == 0 ||
 	    strcmp(name, "phandle") == 0) {
 		assert(size == 4);
-		node->phandle = *(const u32 *)val;
-		if (node->phandle >= last_phandle)
-			last_phandle = node->phandle;
+		node_priv->phandle = *(const u32 *)val;
+		if (node_priv->phandle >= last_phandle)
+			last_phandle = node_priv->phandle;
 		return NULL;
 	}
 
@@ -422,21 +435,23 @@  static u32 dt_property_get_cell(const struct dt_property *prop, u32 index)
 /* First child of this node. */
 static struct pdbg_target *dt_first(const struct pdbg_target *root)
 {
-	return list_top(&root->children, struct pdbg_target, list);
+	return &list_top(&pdbg_target_to_priv(root)->children, struct pdbg_target_priv, list)->target;
 }
 
 /* Return next node, or NULL. */
 static struct pdbg_target *dt_next(const struct pdbg_target *root,
 			const struct pdbg_target *prev)
 {
+	struct pdbg_target_priv *prev_priv = pdbg_target_to_priv(prev);
+
 	/* Children? */
-	if (!list_empty(&prev->children))
+	if (!list_empty(&prev_priv->children))
 		return dt_first(prev);
 
 	do {
 		/* More siblings? */
-		if (prev->list.next != &prev->parent->children.n)
-			return list_entry(prev->list.next, struct pdbg_target,list);
+		if (prev_priv->list.next != &pdbg_target_to_priv(prev->parent)->children.n)
+			return &list_entry(prev_priv->list.next, struct pdbg_target_priv, list)->target;
 
 		/* No more siblings, move up to parent. */
 		prev = prev->parent;
@@ -563,11 +578,11 @@  static int dt_expand_node(struct pdbg_target *node, const void *fdt, int fdt_nod
 			name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
 			if (strcmp("index", name) == 0) {
 				memcpy(&data, prop->data, sizeof(data));
-				node->index = fdt32_to_cpu(data);
+				pdbg_target_to_priv(node)->index = fdt32_to_cpu(data);
 			}
 
 			if (strcmp("status", name) == 0)
-				node->status = str_to_status(prop->data);
+				pdbg_target_to_priv(node)->status = str_to_status(prop->data);
 
 			dt_add_property(node, name, prop->data,
 					fdt32_to_cpu(prop->len));
diff --git a/libpdbg/hwunit.c b/libpdbg/hwunit.c
index c7ec63d..5c40d23 100644
--- a/libpdbg/hwunit.c
+++ b/libpdbg/hwunit.c
@@ -21,6 +21,9 @@ 
 
 #define MAX_HW_UNITS	1024
 
+struct pdbg_target pdbg_target_size;
+uint8_t pdbg_target_size_test[sizeof(struct pdbg_target)];
+
 static const struct hw_unit_info *g_hw_unit[MAX_HW_UNITS];
 static int g_hw_unit_count;
 
diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c
index 0c5a451..bec2488 100644
--- a/libpdbg/libpdbg.c
+++ b/libpdbg/libpdbg.c
@@ -9,6 +9,7 @@  static pdbg_progress_tick_t progress_tick;
 struct pdbg_target *__pdbg_next_target(const char *class, struct pdbg_target *parent, struct pdbg_target *last)
 {
 	struct pdbg_target *next, *tmp;
+	struct pdbg_target_priv *last_priv;
 	struct pdbg_target_class *target_class;
 
 	if (class && !find_target_class(class))
@@ -17,15 +18,17 @@  struct pdbg_target *__pdbg_next_target(const char *class, struct pdbg_target *pa
 	target_class = require_target_class(class);
 
 retry:
+	last_priv = pdbg_target_to_priv(last);
+
 	/* No more targets left to check in this class */
-	if ((last && last->class_link.next == &target_class->targets.n) ||
+	if ((last && last_priv->class_link.next == &target_class->targets.n) ||
 	    list_empty(&target_class->targets))
 		return NULL;
 
 	if (last)
-		next = list_entry(last->class_link.next, struct pdbg_target, class_link);
+		next = &list_entry(last_priv->class_link.next, struct pdbg_target_priv, class_link)->target;
 	else
-		if (!(next = list_top(&target_class->targets, struct pdbg_target, class_link)))
+		if (!(next = &list_top(&target_class->targets, struct pdbg_target_priv, class_link)->target))
 			return NULL;
 
 	if (!parent)
@@ -45,21 +48,24 @@  retry:
 
 struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last)
 {
-	if (!parent || list_empty(&parent->children))
+	struct pdbg_target_priv *parent_priv = pdbg_target_to_priv(parent);
+	struct pdbg_target_priv *last_priv = pdbg_target_to_priv(last);
+
+	if (!parent || list_empty(&parent_priv->children))
 		return NULL;
 
 	if (!last)
-		return list_top(&parent->children, struct pdbg_target, list);
+		return &list_top(&parent_priv->children, struct pdbg_target_priv, list)->target;
 
-	if (last->list.next == &parent->children.n)
+	if (last_priv->list.next == &parent_priv->children.n)
 		return NULL;
 
-	return list_entry(last->list.next, struct pdbg_target, list);
+	return &list_entry(last_priv->list.next, struct pdbg_target_priv, list)->target;
 }
 
 enum pdbg_target_status pdbg_target_status(struct pdbg_target *target)
 {
-	return target->status;
+	return pdbg_target_to_priv(target)->status;
 }
 
 void pdbg_target_status_set(struct pdbg_target *target, enum pdbg_target_status status)
@@ -68,7 +74,7 @@  void pdbg_target_status_set(struct pdbg_target *target, enum pdbg_target_status
 	 * blow up obviously if this happens */
 	assert(status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_MUSTEXIST);
 
-	target->status = status;
+	pdbg_target_to_priv(target)->status = status;
 }
 
 /* Searches up the tree and returns the first valid index found */
@@ -76,12 +82,12 @@  uint32_t pdbg_target_index(struct pdbg_target *target)
 {
 	struct pdbg_target *dn;
 
-	for (dn = target; dn && dn->index == -1; dn = dn->parent);
+	for (dn = target; dn && pdbg_target_to_priv(dn)->index == -1; dn = dn->parent);
 
 	if (!dn)
 		return -1;
 	else
-		return dn->index;
+		return pdbg_target_to_priv(dn)->index;
 }
 
 /* Find a target parent from the given class */
@@ -132,7 +138,7 @@  char *pdbg_target_name(struct pdbg_target *target)
 
 const char *pdbg_target_dn_name(struct pdbg_target *target)
 {
-	return target->dn_name;
+	return pdbg_target_to_priv(target)->dn_name;
 }
 
 int pdbg_target_u32_property(struct pdbg_target *target, const char *name, uint32_t *val)
diff --git a/libpdbg/target.c b/libpdbg/target.c
index 5808d02..ce2bb2d 100644
--- a/libpdbg/target.c
+++ b/libpdbg/target.c
@@ -400,7 +400,7 @@  enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target)
 		case PDBG_TARGET_NONEXISTENT:
 			/* The parent doesn't exist neither does it's
 			 * children */
-			target->status = PDBG_TARGET_NONEXISTENT;
+			pdbg_target_to_priv(target)->status = PDBG_TARGET_NONEXISTENT;
 			return PDBG_TARGET_NONEXISTENT;
 
 		case PDBG_TARGET_DISABLED:
@@ -427,11 +427,11 @@  enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target)
 	if (target->probe && target->probe(target)) {
 		/* Could not find the target */
 		assert(pdbg_target_status(target) != PDBG_TARGET_MUSTEXIST);
-		target->status = PDBG_TARGET_NONEXISTENT;
+		pdbg_target_to_priv(target)->status = PDBG_TARGET_NONEXISTENT;
 		return PDBG_TARGET_NONEXISTENT;
 	}
 
-	target->status = PDBG_TARGET_ENABLED;
+	pdbg_target_to_priv(target)->status = PDBG_TARGET_ENABLED;
 	return PDBG_TARGET_ENABLED;
 }
 
@@ -449,7 +449,7 @@  void pdbg_target_release(struct pdbg_target *target)
 	/* Release the target */
 	if (target->release)
 		target->release(target);
-	target->status = PDBG_TARGET_RELEASED;
+	pdbg_target_to_priv(target)->status = PDBG_TARGET_RELEASED;
 }
 
 /*
@@ -477,10 +477,10 @@  bool pdbg_target_is_class(struct pdbg_target *target, const char *class)
 
 void *pdbg_target_priv(struct pdbg_target *target)
 {
-	return target->priv;
+	return pdbg_target_to_priv(target)->priv;
 }
 
 void pdbg_target_priv_set(struct pdbg_target *target, void *priv)
 {
-	target->priv = priv;
+	pdbg_target_to_priv(target)->priv = priv;
 }
diff --git a/libpdbg/target.h b/libpdbg/target.h
index 2ecbf98..ba2b20d 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -39,19 +39,25 @@  struct pdbg_target {
 	int (*probe)(struct pdbg_target *target);
 	void (*release)(struct pdbg_target *target);
 	uint64_t (*translate)(struct pdbg_target *target, uint64_t addr);
+	struct pdbg_target *parent;
+};
+
+struct pdbg_target_priv {
 	int index;
 	enum pdbg_target_status status;
 	const char *dn_name;
 	struct list_node list;
 	struct list_head properties;
 	struct list_head children;
-	struct pdbg_target *parent;
 	u32 phandle;
 	bool probed;
 	struct list_node class_link;
+	struct pdbg_target target;
 	void *priv;
 };
 
+#define pdbg_target_to_priv(x) (x ? container_of(x, struct pdbg_target_priv, target) : NULL)
+
 struct pdbg_target *require_target_parent(struct pdbg_target *target);
 struct pdbg_target_class *find_target_class(const char *name);
 struct pdbg_target_class *require_target_class(const char *name);