Subject: [Cbor] Review of draft-bormann-cbor-cddl-control-00

From: Jim Schaad <[email-masked]>
Date: Tue, 16 Jun 2020 14:25:31 -0700

* 2.1 - para 3 - I think you can state this easier as "Target can controller
MUST be strings.  The result of the operation has the type of the target.
The concatenation is performed on the bytes in both strings."

* 2.1 - para 3 - I just did a really fast search on CDDL and I am not sure
that it states anyplace that strings are treated as UTF-8.  I think this is
probably implicit but saying you are doing the concatenation on the bytes
has the potential to cause a problem.

* 2.2 - para 2 - The rule for dealing w/ mixed floating point and integer is
ambiguous.   This is not a good idea.  It would seem that an equally valid
interpretation is to use ceiling rather than floor.

* 3 - nit s/can directly be added/are added/

* 3 - need to have some rules on the controls, targets and controller in
terms of types. Can I use .abnf with a binary target and controller?  

* 3 - I assume that there is an implicit rule that the ABNF must be
validated and the CDDL is invalid if it is not valid.

* 4 - I need a much more detailed explanation of what this control is
supposed to be doing.  

* 4 - I find the following very hard to parse: Extensions that are known at
the time this definition is known

Jim

---

From: Carsten Bormann <[email-masked]>
Date: Wed, 17 Jun 2020 03:09:45 +0200

Hi Jim,

thank you for your fast review!

> The concatenation is performed on the bytes in both strings."

Done, thanks.

> has the potential to cause a problem.

Added "If the target is a text string, the result of that concatenation MUST
be valid UTF-8."

> interpretation is to use ceiling rather than floor.

Fixed.

> * 3 - nit s/can directly be added/are added/

Done (is).

> validated and the CDDL is invalid if it is not valid.

Well, we never exactly specified what “valid” means for ABNF.
If you mean that a tool should throw an error upon encountering a controller string that is not “element CRLF *rule”.

> * 4 - I need a much more detailed explanation of what this control is

I added one more way to further muddy the waters (but also adding explanation).

> the time this definition is known

Fixed (I think).

Thanks again; I have submitted the update as -01.

Grüße, Carsten