clicon / clixon

YANG-based toolchain including NETCONF and RESTCONF interfaces and an interactive CLI
http://www.clicon.org/
Other
215 stars 72 forks source link

More specific error shall be provided when there is a type def on top of default types #319

Closed navaneethyv closed 1 year ago

navaneethyv commented 2 years ago

Hi,

Please consider the following typedef.

 typedef mpls-label {
     type uint16 {
         range "0 .. 255";
     }
     description "Mpls label value should be in the range 0 - 255";
 }

The actual schema

  list ipv4
  {
      key "prefix4";
      description
          "Configure ipv4 address under sub-interface."
       leaf prefix4 {
          type inet:ipv4-prefix;
          description
              "Configure ipv4 address.";
       }
      leaf label {
          type mpls-label;
          description
              "Configure label associated with the address.";
      }
  }
cli> set  ipv4 4.4.4.4/32 label 2222
CLI syntax Error : Number 2222 out of range: 0 - 25

When you cross the actual underlying type range, We see a different error.

cli> set  ipv4 4.4.4.4/32 label 22222222222
CLI syntax Error : Number 2222222222 out of range: 0 - 65535

The second error is not the most specific error. Can we print the most specific error when a range / typedef is present ?

olofhagsand commented 2 years ago

Code explanation: in match_variable() here: https://github.com/clicon/cligen/blob/c450b310b0e149d29339cee2b6034f0fb05d90aa/cligen_match.c#L107 there is first a parsing into the base type (uint16) and then validation where extra ranges are checked:

  cv_parse1(str, cv, reason); 
  cv_validate(h, cv, cs, co->co_command, reason);

In the example's first case for 2222, the parsing succeeds but the validation fails. In the second case for 22222222222, the parsing fails.

navaneethyv commented 2 years ago

Hmm. Would it make sense to parse every INT variant as uint64 and then feed a fixed INT and INT or use the user defined range inside cv_validate for common validation errors ?

olofhagsand commented 2 years ago

It is do-able but got somewhat complex

olofhagsand commented 1 year ago

Fixed in cligen by parsing twice for int:s less than 64-bits. Eg:

  1. parse as 64-bit
  2. validate
  3. parse as < 64-bit