Subject: RE: [rohc] WGLC comments to torture-tests and user-guide

From: "Surtees, Abigail" <[email-masked]>
Date: Mon, 22 Aug 2005 17:19:57 +0100

Hi Pekka,

Thanks for the comments.  A couple of comments in-line:
> 
> I've run successfully all the tests on torture-tests-01 spending
> indicated number of cycles. I have brewsed through the document
> otherwise. I've found a few typos:
> 
> sucessively => successively ?, 
> begining => beginning,
> partial_identifer_length => partial_identifier_length

Agreed.
> 
> Random late improvement: it would be useful to state why a
> decompression failure occurs in appendix A, too.
> 
I think this would lead to quite a lot of duplication of information
within the document, so I'm inclined to say I don't think it's
necessary.  It would also need lots of checking to ensure there aren't
contradictions between tests and appendix.

> I've assembled and run the algorithms specified in user-guide, and
> got the expected output.  
> 
> Nits:
> 
> 1) The assembly language ABNF for <name> is incorrect,
> 
>    name             =     (lowercase / "_") 1*(lowercase / 
> digit / "_")
> 
> should be
> 
>    name             =     (lowercase / "_") 0*(lowercase / 
> digit / "_")
> 
Yes, agreed

> (After spotting this significant error. I trust authors to 
> run spelling
> checker on their document.)
> 
OK!

> 2) This paragraph could mention readonly(1):
> 
>    N.B. When using "!" in the assembly language to generate bytecode,
>    care must be taken to ensure that the address of the zero used at
>    bytecode generation time will still contain zero when the 
> bytecode is
>    run.
> 
Agreed.

> Also, it would be nice to clarify what is the area of memory the
> readonly() directive acts on in section 3.2.3
> 
Agreed.  How about:

"The default assumption is that all memory can be changed during
execution of the UDVM.  That is, that unless stated otherwise,  memory
is assumed to be readonly (0).  The area of memory covered by a readonly
(1) directive is the area between that directive and either a readonly
(0) directive or the end of the bytecode (whichever is the smaller
area)."

> 3) Last paragraph in 3.2.2 could expressly state that the
> resulting bytecode may or need not to be the shortest one.
> 
Yes:

"The length of the resulting bytecode is dependent on the parser used.
Bytecode can be correct and useable without being the smallest possible
representation."

> 4) Section 3.3, a leftover paragraph from days when destionation
>   value 0 was not reserved for NACK?
> 
>    Additionally, all memory addresses between Address 0 and Address 31
>    inclusive are initialized to endpoint-specific values by 
> the UDVM, so
>    they must be specified as padding in the bytecode or the standard
>    SigComp header cannot be used.  Memory addresses from Address 32 to
>    Address (destination - 1) inclusive are initialized to 0, so they
>    must be specified either as padding or as 0s if the 
> bytecode is to be
>    successfully uploaded using the standard SigComp header.
> 
I don't think so.  All bytes between 32 and where the bytecode is put
are assumed to contain zero - expressed as padding in the mnemonic code.
It is the code-len field in the SigComp message that NACK uses, not the
destination field.

> 5) Section 4.1.1.1 following paragraph could mention "stream" 
> or "tcp":
> 
>    Note that the actual size of this circular buffer depends on the
>    total amount of memory available to the UDVM.  The minimum size of
>    the UDVM memory is 1K, so the circular buffer will always 
> contain at
>    least 512 bytes.

I think this is a misleading paragraph and suggest we remove it.  The
size of the buffer depends on how you chose to define it at the
compressor.

> 
> 6) this draft => this document in a few places

OK.  I'll fix those
> 
> Random late improvement: unknown assembly directives should 
> be ignored.

I'm not sure that unknown directives should necessarily be ignored -
there are some e.g. readonly that are a help to a compiler/parser but
others e.g. byte define specific behaviour for compilation.  I think the
important ones are covered by this draft.  If users want to make up
additional ones to help their particular compile they can, but I don't
think they (or the fact that there could be others) need to be in this
document.  What do you think?

Best regards,

Abbie

> 
> --Pekka
> 
> _______________________________________________
> Rohc mailing list
> [email-masked]
> https://www1.ietf.org/mailman/listinfo/rohc
> 

_______________________________________________
Rohc mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/rohc

---

From: Pekka Pessi <[email-masked]>
Date: Mon, 22 Aug 2005 20:40:58 +0300

>contradictions between tests and appendix.

What I'd like to see in appendix A would be low-level reason (a'la
RFC 4077 reason code), The point of providing reason codes in
sigcomp-torture-tests is the amount of checking, should everyone
running torture-tests do it, or should it be done only once.

>(0) directive or the end of the bytecode (whichever is the smaller

OK, but then you need to add readonly(1) to all example algorithms
using !. What about:

   The default assumption is that all memory outside bytecode can be
   changed during execution of the UDVM. That is, that unless stated
   otherwise, memory from 0 to (destination - 1) is assumed to be
   readonly (0), memory area from (destination) to the end of the
   bytecode is readonly (1), and memory from the end of bytecode to
   the end of udvm memory is is readonly(0). The area of memory
   covered by a readonly directive is the area between that directive
   and either an another readonly directive or the end of the
   bytecode (whichever is the smaller area).

>Bytecode can be correct and useable without being the smallest possible

OK. Also replace [parser] "inserts the shortest bytecode capable
of encoding" => "usually inserts the shortest bytecode capable of
encoding".

>document.  What do you think?

I was thinking that a UDVM assembler should have a way to support
non-standard directives.  So,  define

   directive /= unknown-directive

   unknown-directive = name ["(" expression *("," ; expression) ")"]

and the parser should ignore all <unknown-directive>. 

Or define

   directive /= private-directive

   private-directive = "__" name ["(" expression *("," ; expression) ")"]

and the parser should ignore all <private-directive>. 

--Pekka