On Fri, Mar 23, 2018 at 08:30:38AM -0600, Simon Glass wrote:
> Hi Grant,
>
> On 20 March 2018 at 21:45, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
> > From: Grant Likely <grant@xxxxxxxxxxxxxxxxxxxxxxxxx>
> >
> > If datatype markers are present in the property value, use them to
> > output the data in the correct format instead of trying to guess the
> > datatype. This also will preserve data grouping, such as in an
> > interrupts list.
> >
> > This is a step forward for preserving and using datatype information
> > when processing DTS/DTB files. Schema validation tools can use the
> > datatype information to make sure a DT is correctly formed and
> > intepreted.
> >
> > Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxxxx>
> > ---
> > dtc.h | 1 +
> > treesource.c | 227 ++++++++++++++++++++++++++++++++---------------------------
> > 2 files changed, 125 insertions(+), 103 deletions(-)
> >
>
> I think this should have a test.
I agree.
>
> [...]
>
> > +const char * delim_start[] = {
>
> A few places in this patch the * appears in a different place from the
> rest of the code.
Right dtc code style (which is basically == Linux kernel style) is to
use "type *variable".
>
> > + [MARKER_UINT8] = "[",
> > + [MARKER_UINT16] = "/bits/ 16 <",
> > + [MARKER_UINT32] = "<",
> > + [MARKER_UINT64] = "/bits/ 64 <",
> > + [MARKER_STRING] = "",
> > +};
> > +const char * delim_end[] = {
> > + [MARKER_UINT8] = " ]",
> > + [MARKER_UINT16] = " >",
> > + [MARKER_UINT32] = " >",
> > + [MARKER_UINT64] = " >",
> > + [MARKER_STRING] = "",
> > +};
> > static void write_propval(FILE *f, struct property *prop)
> > {
> > - int len = prop->val.len;
> > - const char *p = prop->val.val;
> > - struct marker *m = prop->val.markers;
> > + size_t len = prop->val.len;
> > + size_t chunk_len;
> > + struct marker *next_type, *m = prop->val.markers;
> > + struct marker dummy_marker;
> > int nnotstring = 0, nnul = 0;
> > int nnotstringlbl = 0, nnotcelllbl = 0;
> > int i;
> > + enum markertype emit_type = MARKER_NONE;
> >
> > if (len == 0) {
> > fprintf(f, ";\n");
> > return;
> > }
> >
> > - for (i = 0; i < len; i++) {
> > - if (! isstring(p[i]))
> > - nnotstring++;
> > - if (p[i] == '\0')
> > - nnul++;
> > - }
> > + fprintf(f, " = ");
> > +
> > + next_type = next_type_marker(m);
> > + chunk_len = (next_type ? next_type->offset : len);
> > + if (chunk_len) {
> > + /* data type information missing, need to guess */
> > + const char *p = prop->val.val;
> > + for (i = 0; i < chunk_len; i++) {
> > + if (! isstring(p[i]))
> > + nnotstring++;
> > + if (p[i] == '\0')
> > + nnul++;
> > + }
> > +
> > + for_each_marker_of_type(m, LABEL) {
> > + if (m->offset >= chunk_len)
> > + break;
> > + if ((m->offset > 0) && (prop->val.val[m->offset - 1] != '\0'))
> > + nnotstringlbl++;
> > + if ((m->offset % sizeof(cell_t)) != 0)
> > + nnotcelllbl++;
> > + }
> >
> > - 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[chunk_len-1] == '\0') && (nnotstring == 0) && (nnul < (chunk_len-nnul))
> > + && (nnotstringlbl == 0)) {
> > + dummy_marker.type = MARKER_STRING;
> > + } else if (((chunk_len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
> > + dummy_marker.type = MARKER_UINT32;
> > + } else {
> > + dummy_marker.type = MARKER_UINT8;
> > + }
> > +
> > + dummy_marker.next = prop->val.markers;
> > + dummy_marker.offset = 0;
> > + dummy_marker.ref = NULL;
>
> So is the dummy marker used to hold the guess? Could the output code
> go for a particularly type and value go in a separate function
> instead?
>
> > + m = &dummy_marker;
> > }
> >
> > - fprintf(f, " = ");
> > - if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
> > - && (nnotstringlbl == 0)) {
> > - write_propval_string(f, prop->val);
> > - } else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
> > - write_propval_cells(f, prop->val);
> > - } else {
> > - write_propval_bytes(f, prop->val);
> > + for_each_marker(m) {
> > + chunk_len = (m->next ? m->next->offset : len) - m->offset;
> > + const char *p = &prop->val.val[m->offset];
> > +
> > + switch(m->type) {
> > + case MARKER_UINT8:
> > + case MARKER_UINT16:
> > + case MARKER_UINT32:
> > + case MARKER_UINT64:
> > + case MARKER_STRING:
> > + if (emit_type != MARKER_NONE)
> > + fprintf(f, "%s, ", delim_end[emit_type]);
> > + emit_type = m->type;
> > + fprintf(f, "%s", delim_start[emit_type]);
> > + break;
> > + case LABEL:
> > + fprintf(f, " %s:", m->ref);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + if (chunk_len <= 0)
> > + continue;
> > +
> > + switch(emit_type) {
> > + case MARKER_UINT16:
> > + write_propval_int(f, p, chunk_len, 2);
> > + break;
> > + case MARKER_UINT32:
> > + write_propval_int(f, p, chunk_len, 4);
> > + break;
> > + case MARKER_UINT64:
> > + write_propval_int(f, p, chunk_len, 8);
> > + break;
> > + case MARKER_STRING:
> > + write_propval_string(f, p, chunk_len);
> > + break;
> > + default:
> > + write_propval_int(f, p, chunk_len, 1);
> > + }
> > }
> >
> > - fprintf(f, ";\n");
> > + fprintf(f, "%s;\n", delim_end[emit_type] ? : "");
> > }
> >
> > static void write_tree_source_node(FILE *f, struct node *tree, int level)
>
> Regards,
> Simon
--
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
