Re: [PATCH 2/2] Add support for YAML encoded output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Thu, Jul 12, 2018 at 06:20:06PM -0600, Rob Herring wrote:
> From: Grant Likely <grant.likely@xxxxxxx>
> 
> YAML encoded DT is useful for validation of DTs using binding schemas.
> 
> The YAML encoding is an intermediate format used for validation and
> is therefore subject to change as needed. The YAML output is dependent
> on DTS input with type information preserved.
> 
> Signed-off-by: Grant Likely <grant.likely@xxxxxxx>
> [robh: make YAML support optional, build fixes, Travis CI test,
>  preserve type information in paths and phandles]
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> ---
>  .travis.yml              |  10 ++
>  Documentation/manual.txt |   3 +
>  Makefile                 |   8 +-
>  Makefile.dtc             |   4 +
>  dtc.c                    |  11 ++
>  dtc.h                    |   4 +
>  yamltree.c               | 290 +++++++++++++++++++++++++++++++++++++++
>  7 files changed, 329 insertions(+), 1 deletion(-)
>  create mode 100644 yamltree.c
> 
> diff --git a/.travis.yml b/.travis.yml
> index 87adfa092b5d..86ab2053eca4 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -33,3 +33,13 @@ matrix:
>        script:
>          - make
>          - make check
> +
> +    # Check it builds properly with the yaml bits
> +    - addons:
> +        apt:
> +          packages:
> +            - valgrind
> +            - libyaml-0-2
> +      script:
> +        - make
> +        - make check

I don't think adding another build is the best approach here.  Instead
it would be better to include the YAML stuff in the first "all bells
and whistles" build.  The second build will already check that things
work correctly when the yaml (and python) stuff is disabled.  We might
want to consider builds for yaml-but-no-python and python-but-no-yaml
but I think that's relatively unlikely to find problems.

> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 6898caa5b40f..db32dd72a0fc 100644
> --- a/Documentation/manual.txt
> +++ b/Documentation/manual.txt
> @@ -78,6 +78,9 @@ The currently supported Output Formats are:
>          then simply be added to your Makefile.  Additionally, the
>          assembly file exports some symbols that can be used.
>  
> +     - "yaml": DT encoded in YAML format. This representation is an
> +       intermediate format used for validation tools.
> +
>  
>  3) Command Line
>  
> diff --git a/Makefile b/Makefile
> index 6d55e136a0a8..4694648c9de7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -39,6 +39,12 @@ INCLUDEDIR = $(PREFIX)/include
>  HOSTOS := $(shell uname -s | tr '[:upper:]' '[:lower:]' | \
>  	    sed -e 's/\(cygwin\|msys\).*/\1/')
>  
> +HAS_YAML := $(shell ! $(PKG_CONFIG) --exists yaml-0.1; echo $$?)
> +ifeq ($(HAS_YAML),1)
> + CFLAGS += -DHAS_YAML
> + LDLIBS += $(shell $(PKG_CONFIG) --silence-errors --libs yaml-0.1)
> +endif
> +

For consistency with the python stuff (our only current optional
feature), I'd prefer to invert the sense here.  Basically assume YAML
support by default, and disable it with a NO_YAML option.

>  ifeq ($(HOSTOS),darwin)
>  SHAREDLIB_EXT     = dylib
>  SHAREDLIB_CFLAGS  = -fPIC
> @@ -322,7 +328,7 @@ clean: libfdt_clean pylibfdt_clean tests_clean
>  #
>  %: %.o
>  	@$(VECHO) LD $@
> -	$(LINK.c) -o $@ $^
> +	$(LINK.c) -o $@ $^ $(LDLIBS)
>  
>  %.o: %.c
>  	@$(VECHO) CC $@
> diff --git a/Makefile.dtc b/Makefile.dtc
> index bece49b35535..b021a9e0bda3 100644
> --- a/Makefile.dtc
> +++ b/Makefile.dtc
> @@ -14,5 +14,9 @@ DTC_SRCS = \
>  	treesource.c \
>  	util.c
>  
> +ifeq ($(HAS_YAML),1)
> +DTC_SRCS += yamltree.c
> +endif
> +
>  DTC_GEN_SRCS = dtc-lexer.lex.c dtc-parser.tab.c
>  DTC_OBJS = $(DTC_SRCS:%.c=%.o) $(DTC_GEN_SRCS:%.c=%.o)
> diff --git a/dtc.c b/dtc.c
> index c36994e6eac5..07e5b3a88c26 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -95,6 +95,9 @@ static const char * const usage_opts_help[] = {
>  	"\n\tOutput formats are:\n"
>  	 "\t\tdts - device tree source text\n"
>  	 "\t\tdtb - device tree blob\n"
> +#ifdef HAS_YAML
> +	 "\t\tyaml - device tree encoded as YAML\n"
> +#endif
>  	 "\t\tasm - assembler source",
>  	"\n\tBlob version to produce, defaults to "stringify(DEFAULT_FDT_VERSION)" (for dtb and asm output)",
>  	"\n\tOutput dependency file",
> @@ -128,6 +131,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
>  		return fallback;
>  	if (!strcasecmp(s, ".dts"))
>  		return "dts";
> +	if (!strcasecmp(s, ".yaml"))
> +		return "yaml";
>  	if (!strcasecmp(s, ".dtb"))
>  		return "dtb";
>  	return fallback;
> @@ -350,6 +355,12 @@ int main(int argc, char *argv[])
>  
>  	if (streq(outform, "dts")) {
>  		dt_to_source(outf, dti);
> +#ifdef HAS_YAML
> +	} else if (streq(outform, "yaml")) {
> +		if (!streq(inform, "dts"))
> +			die("YAML output format requires dts input format\n");
> +		dt_to_yaml(outf, dti);
> +#endif
>  	} else if (streq(outform, "dtb")) {
>  		dt_to_blob(outf, dti, outversion);
>  	} else if (streq(outform, "asm")) {
> diff --git a/dtc.h b/dtc.h
> index 303c2a6a73b7..7debecfdb46d 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -298,6 +298,10 @@ struct dt_info *dt_from_blob(const char *fname);
>  void dt_to_source(FILE *f, struct dt_info *dti);
>  struct dt_info *dt_from_source(const char *f);
>  
> +/* YAML source */
> +
> +void dt_to_yaml(FILE *f, struct dt_info *dti);
> +
>  /* FS trees */
>  
>  struct dt_info *dt_from_fs(const char *dirname);
> diff --git a/yamltree.c b/yamltree.c
> new file mode 100644
> index 000000000000..b6de744c92f3
> --- /dev/null
> +++ b/yamltree.c
> @@ -0,0 +1,290 @@
> +/*
> + * (C) Copyright Linaro, Ltd. 2018
> + * (C) Copyright Arm Holdings.  2017
> + * (C) Copyright David Gibson <dwg@xxxxxxxxxxx>, IBM Corporation.  2005.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
> + *                                                                   USA
> + */
> +
> +#include <stdlib.h>
> +#include <yaml.h>
> +#include "dtc.h"
> +#include "srcpos.h"
> +
> +char *yaml_error_name[] = {
> +	[YAML_NO_ERROR] = "no error",
> +	[YAML_MEMORY_ERROR] = "memory error",
> +	[YAML_READER_ERROR] = "reader error",
> +	[YAML_SCANNER_ERROR] = "scanner error",
> +	[YAML_PARSER_ERROR] = "parser error",
> +	[YAML_COMPOSER_ERROR] = "composer error",
> +	[YAML_WRITER_ERROR] = "writer error",
> +	[YAML_EMITTER_ERROR] = "emitter error",
> +};
> +
> +#define yaml_die(emitter) die("yaml '%s': %s in %s, line %i\n", yaml_error_name[(emitter)->error], (emitter)->problem, __func__, __LINE__)
> +
> +static void yaml_propval_phandle_args(yaml_emitter_t *emitter, char *data, int len)
> +{
> +	yaml_event_t event;
> +	char *dataend = data + len;
> +	char *tag = "!phandle";
> +	char buf[32];
> +
> +	if (len % 4) {
> +		die("Property data length %i isn't a multiple of 4\n", len);
> +		return;
> +	}
> +
> +	yaml_sequence_start_event_initialize(&event, NULL,
> +		(yaml_char_t *)NULL, 1, YAML_FLOW_SEQUENCE_STYLE);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);

For a library that's supposed to make yaml easy, that's pretty damn
verbose, but whatever :/.

> +
> +	sprintf(buf, "0x%"PRIx32, fdt32_to_cpu(*(fdt32_t*)data));
> +	yaml_scalar_event_initialize(&event, NULL,
> +		(yaml_char_t*)tag, (yaml_char_t *)buf,
> +		strlen(buf), 0, 0, YAML_PLAIN_SCALAR_STYLE);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +
> +	for (data += 4; data < dataend; data += 4) {
> +		sprintf(buf, "0x%"PRIx32, fdt32_to_cpu(*(fdt32_t*)data));
> +
> +		yaml_scalar_event_initialize(&event, NULL,
> +			(yaml_char_t*)NULL, (yaml_char_t *)buf,
> +			strlen(buf), 1, 1, YAML_PLAIN_SCALAR_STYLE);
> +		if (!yaml_emitter_emit(emitter, &event))
> +			yaml_die(emitter);
> +	}
> +
> +	yaml_sequence_end_event_initialize(&event);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +}
> +
> +static void yaml_propval_int(yaml_emitter_t *emitter, char *data, int len, int width)
> +{
> +	yaml_event_t event;
> +	char *dataend = data + len;
> +	void *tag;
> +	char buf[32];
> +
> +	if (len % width) {
> +		fprintf(stderr, "Warning: Property data length %i isn't a multiple of %i\n", len, width);

a) don't use a raw fprintf(), and b) this can be an assert(), no -
surely getting here means a bug in the caller doesn't it?

> +		width = 1;
> +	}
> +
> +	switch(width) {
> +		case 1: tag = "!u8"; break;
> +		case 2: tag = "!u16"; break;
> +		case 4: tag = "!u32"; break;
> +		case 8: tag = "!u64"; break;
> +		default:
> +			die("Invalid width %i", width);
> +	}
> +
> +	yaml_sequence_start_event_initialize(&event, NULL,
> +		(yaml_char_t *)tag, width == 4, YAML_FLOW_SEQUENCE_STYLE);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +
> +	for (; data < dataend; data += width) {
> +		switch(width) {
> +		case 1:
> +			sprintf(buf, "0x%"PRIx8, *(uint8_t*)data);
> +			break;
> +		case 2:
> +			sprintf(buf, "0x%"PRIx16, fdt16_to_cpu(*(fdt16_t*)data));
> +			break;
> +		case 4:
> +			sprintf(buf, "0x%"PRIx32, fdt32_to_cpu(*(fdt32_t*)data));
> +			break;
> +		case 8:
> +			sprintf(buf, "0x%"PRIx64, fdt64_to_cpu(*(fdt64_t*)data));
> +			break;
> +		}
> +
> +		yaml_scalar_event_initialize(&event, NULL,
> +			(yaml_char_t*)YAML_INT_TAG, (yaml_char_t *)buf,
> +			strlen(buf), 1, 1, YAML_PLAIN_SCALAR_STYLE);
> +		if (!yaml_emitter_emit(emitter, &event))
> +			yaml_die(emitter);
> +	}
> +
> +	yaml_sequence_end_event_initialize(&event);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +}
> +
> +static void yaml_propval_string(yaml_emitter_t *emitter, char *str, int len)
> +{
> +	yaml_event_t event;
> +	int i;
> +
> +	assert(str[len-1] == '\0');
> +
> +	/* Make sure the entire string is in the lower 7-bit ascii range */
> +	for (i = 0; i < len; i++) {
> +		if (!isascii(str[i])) {
> +			fprintf(stderr, "Warning: non-ASCII character(s) in property string\n");

Again, a raw printf isn't great.

> +			yaml_propval_int(emitter, str, len, 1);
> +			return;
> +		}
> +	}
> +
> +	yaml_scalar_event_initialize(&event, NULL,
> +		(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)str,
> +		len-1, 0, 1, YAML_DOUBLE_QUOTED_SCALAR_STYLE);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +}
> +
> +static void yaml_propval(yaml_emitter_t *emitter, struct property *prop)
> +{
> +	yaml_event_t event;
> +	int len = prop->val.len;
> +	struct marker *m = prop->val.markers;
> +
> +	/* Emit the property name */
> +	yaml_scalar_event_initialize(&event, NULL,
> +		(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)prop->name,
> +		strlen(prop->name), 1, 1, YAML_PLAIN_SCALAR_STYLE);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +
> +	/* Boolean properties are easiest to deal with. Length is zero, so just emit 'true' */
> +	if (len == 0) {
> +		yaml_scalar_event_initialize(&event, NULL,
> +			(yaml_char_t *)YAML_BOOL_TAG,
> +			(yaml_char_t*)"true",
> +			strlen("true"), 1, 0, YAML_PLAIN_SCALAR_STYLE);
> +		if (!yaml_emitter_emit(emitter, &event))
> +			yaml_die(emitter);
> +		return;
> +	}
> +
> +	if (!m)
> +		die("No markers present in property '%s' value\n", prop->name);
> +
> +	yaml_sequence_start_event_initialize(&event, NULL,
> +		(yaml_char_t *)YAML_SEQ_TAG, 1, YAML_FLOW_SEQUENCE_STYLE);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +
> +	for_each_marker(m) {
> +		int chunk_len = (m->next ? m->next->offset : len) - m->offset;
> +		char *data = &prop->val.val[m->offset];
> +
> +		if (chunk_len <= 0)
> +			continue;
> +
> +		switch(m->type) {
> +		case REF_PHANDLE:
> +			yaml_propval_phandle_args(emitter, data, chunk_len);

Handling the phandle and the "args" together seems dubious to me,
since that connection isn't really built into the tree structure, just
a common binding pattern.  Can't you just deal with the phandle, then
deal with the next bit using the type markers just as you would
anyway?

> +			break;
> +		case TYPE_UINT16:
> +			yaml_propval_int(emitter, data, chunk_len, 2);
> +			break;
> +		case TYPE_UINT32:
> +			yaml_propval_int(emitter, data, chunk_len, 4);
> +			break;
> +		case TYPE_UINT64:
> +			yaml_propval_int(emitter, data, chunk_len, 8);
> +			break;
> +		case REF_PATH:
> +		case TYPE_STRING:

AFAIK a path reference will generate both a REF_PATH and a TYPE_STRING
marker, so won't this try to emit something 0-length followed by the
actual output you ant?

> +			yaml_propval_string(emitter, data, chunk_len);
> +			break;
> +		default:
> +			yaml_propval_int(emitter, data, chunk_len, 1);
> +			break;
> +		}
> +	}
> +
> +	yaml_sequence_end_event_initialize(&event);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +}
> +
> +
> +static void yaml_tree(struct node *tree, yaml_emitter_t *emitter)
> +{
> +	struct property *prop;
> +	struct node *child;
> +	yaml_event_t event;
> +
> +	if (tree->deleted)
> +		return;
> +
> +	yaml_mapping_start_event_initialize(&event, NULL,
> +		(yaml_char_t *)YAML_MAP_TAG, 1, YAML_ANY_MAPPING_STYLE);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +
> +	for_each_property(tree, prop)
> +		yaml_propval(emitter, prop);
> +
> +	/* Loop over all the children, emitting them into the map */
> +	for_each_child(tree, child) {
> +		yaml_scalar_event_initialize(&event, NULL,
> +			(yaml_char_t *)YAML_STR_TAG, (yaml_char_t*)child->name,
> +			strlen(child->name), 1, 0, YAML_PLAIN_SCALAR_STYLE);
> +		if (!yaml_emitter_emit(emitter, &event))
> +			yaml_die(emitter);
> +		yaml_tree(child, emitter);
> +	}
> +
> +	yaml_mapping_end_event_initialize(&event);
> +	if (!yaml_emitter_emit(emitter, &event))
> +		yaml_die(emitter);
> +}
> +
> +void dt_to_yaml(FILE *f, struct dt_info *dti)
> +{
> +	yaml_emitter_t emitter;
> +	yaml_event_t event;
> +
> +	yaml_emitter_initialize(&emitter);
> +	yaml_emitter_set_output_file(&emitter, f);
> +	yaml_stream_start_event_initialize(&event, YAML_UTF8_ENCODING);
> +	if (!yaml_emitter_emit(&emitter, &event))
> +		yaml_die(&emitter);
> +
> +	yaml_document_start_event_initialize(&event, NULL, NULL, NULL, 0);
> +	if (!yaml_emitter_emit(&emitter, &event))
> +		yaml_die(&emitter);
> +
> +	yaml_sequence_start_event_initialize(&event, NULL, (yaml_char_t *)YAML_SEQ_TAG, 1, YAML_ANY_SEQUENCE_STYLE);
> +	if (!yaml_emitter_emit(&emitter, &event))
> +		yaml_die(&emitter);
> +
> +	yaml_tree(dti->dt, &emitter);
> +
> +	yaml_sequence_end_event_initialize(&event);
> +	if (!yaml_emitter_emit(&emitter, &event))
> +		yaml_die(&emitter);
> +
> +	yaml_document_end_event_initialize(&event, 0);
> +	if (!yaml_emitter_emit(&emitter, &event))
> +		yaml_die(&emitter);
> +
> +	yaml_stream_end_event_initialize(&event);
> +	if (!yaml_emitter_emit(&emitter, &event))
> +		yaml_die(&emitter);
> +
> +	yaml_emitter_delete(&emitter);
> +}

-- 
David Gibson			| 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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux