Re: [PATCH 3/6] Improve type guessing when compiling to dts format

David Gibson <[email protected]> Thu, 21 Aug 2025 14:24:50 +1000
Newsgroups	org.kernel.vger.devicetree-compiler
Message-ID	<aKafkjp1r5fUKYEF@zatzit>
On Wed, Aug 20, 2025 at 03:11:29PM +0200, Uwe Kleine-König wrote:
> In the presence of (non-type) markers guess the type of each chunk
> between markers individually instead of only once for the whole
> property.
> 
> Note that this only gets relevant with the next few commits that restore
> labels and phandles. Note further that this rework is necessary with
> these further changes, because phandle markers are currently not
> considered for type guessing and so a phandle at an offset that isn't a
> multiple of 4 triggers an assertion if the property was guessed to have
> type TYPE_UINT32.
> 
> Now that guess_value_type() is only called for data chunks without
> markers, the function can be simplified a bit.
> 
> Signed-off-by: Uwe Kleine-König <[email protected]>

Reviewed-by: David Gibson <[email protected]>

> ---
>  treesource.c | 70 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/treesource.c b/treesource.c
> index 72d1cb5508b5..576495902f0d 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -173,23 +173,20 @@ static struct marker **add_marker(struct marker **mi,
>  	return &nm->next;
>  }
>  
> -static void add_string_markers(struct property *prop)
> +static void add_string_markers(struct property *prop, unsigned int offset, int len)
>  {
> -	int l, len = prop->val.len;
> -	const char *p = prop->val.val;
> +	int l;
> +	const char *p = prop->val.val + offset;
>  	struct marker **mi = &prop->val.markers;
>  
>  	for (l = strlen(p) + 1; l < len; l += strlen(p + l) + 1)
> -		mi = add_marker(mi, TYPE_STRING, l, NULL);
> +		mi = add_marker(mi, TYPE_STRING, offset + l, NULL);
>  }
>  
> -static enum markertype guess_value_type(struct property *prop)
> +static enum markertype guess_value_type(struct property *prop, unsigned int offset, int len)
>  {
> -	int len = prop->val.len;
> -	const char *p = prop->val.val;
> -	struct marker *m = prop->val.markers;
> +	const char *p = prop->val.val + offset;
>  	int nnotstring = 0, nnul = 0;
> -	int nnotstringlbl = 0, nnotcelllbl = 0;
>  	int i;
>  
>  	for (i = 0; i < len; i++) {
> @@ -199,30 +196,49 @@ static enum markertype guess_value_type(struct property *prop)
>  			nnul++;
>  	}
>  
> -	for_each_marker_of_type(m, LABEL) {
> -		if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
> -			nnotstringlbl++;
> -		if ((m->offset % sizeof(cell_t)) != 0)
> -			nnotcelllbl++;
> -	}
> -
> -	if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul <= (len-nnul))
> -	    && (nnotstringlbl == 0)) {
> +	if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul <= len - nnul)) {
>  		if (nnul > 1)
> -			add_string_markers(prop);
> +			add_string_markers(prop, offset, len);
>  		return TYPE_STRING;
> -	} else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
> +	} else if ((len % sizeof(cell_t)) == 0) {
>  		return TYPE_UINT32;
>  	}
>  
>  	return TYPE_UINT8;
>  }
>  
> +static void guess_type_markers(struct property *prop)
> +{
> +	struct marker **m = &prop->val.markers;
> +	unsigned int offset = 0;
> +
> +	for (m = &prop->val.markers; *m; m = &((*m)->next)) {
> +		if (is_type_marker((*m)->type))
> +			/* assume the whole property is already marked */
> +			return;
> +
> +		if ((*m)->offset > offset) {
> +			m = add_marker(m, guess_value_type(prop, offset, (*m)->offset - offset),
> +				       offset, NULL);
> +
> +			offset = (*m)->offset;
> +		}
> +
> +		if ((*m)->type == REF_PHANDLE) {
> +			m = add_marker(m, TYPE_UINT32, offset, NULL);
> +			offset += 4;
> +		}
> +	}
> +
> +	if (offset < prop->val.len)
> +		add_marker(m, guess_value_type(prop, offset, prop->val.len - offset),
> +			   offset, NULL);
> +}
> +
>  static void write_propval(FILE *f, struct property *prop)
>  {
>  	size_t len = prop->val.len;
> -	struct marker *m = prop->val.markers;
> -	struct marker dummy_marker;
> +	struct marker *m;
>  	enum markertype emit_type = TYPE_NONE;
>  	char *srcstr;
>  
> @@ -241,14 +257,8 @@ static void write_propval(FILE *f, struct property *prop)
>  
>  	fprintf(f, " =");
>  
> -	if (!next_type_marker(m)) {
> -		/* data type information missing, need to guess */
> -		dummy_marker.type = guess_value_type(prop);
> -		dummy_marker.next = prop->val.markers;
> -		dummy_marker.offset = 0;
> -		dummy_marker.ref = NULL;
> -		m = &dummy_marker;
> -	}
> +	guess_type_markers(prop);
> +	m = prop->val.markers;
>  
>  	for_each_marker(m) {
>  		size_t chunk_len = (m->next ? m->next->offset : len) - m->offset;

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson
signature.asc (application/pgp-signature, 833 B)