Re: [PATCH] Added a JFFS2 pickle

[Jose E. Marchesi]

> I can think on two strategies for turning JFFS2_Sum_Rec into an agnostic
> type:
>
> a) Rewrite the method to access the fields in the union instead of doing
>    a mapping:
>
>    method get_node_type = uint<16>:
>    {
>      try return dirent.node_type; catch if E_elem {}
>      try return inode.node_type; catch if E_elem {}
>      ...
>      return unknown.node_type;
>    }
>
>    Note how I am relying on the field name instead of the value of
>    node_type.  This is so the union tag logic doesn't get replicated in
>    the method.  The only thing the method assumes is that `unknown' is
>    the last alternative.
>
> b) Use a pinned struct:
>
>    type JFFS2_Sum_Rec =
>      pinned struct
>      {
>        uint<16> node_type;
>
>        union {
>            JFFS2_Sum_Dirent dirent : node_type == JFFS2_NODETYPE_DIRENT;
>            JFFS2_Sum_Inode inode : node_type == JFFS2_NODETYPE_INODE;
>            JFFS2_Sum_XAttr xattr : node_type == JFFS2_NODETYPE_XATTR;
>            JFFS2_Sum_XRef xref : node_type == JFFS2_NODETYPE_XREF;
>            JFFS2_Sum_Unk unknown;
>        } data;
>       };
>
>     or, alternatively, a label in `data':
>
>      type JFFS2_Sum_Rec =
>        struct
>        {
>          uint<16> node_type;
>
>          union {
>              JFFS2_Sum_Dirent dirent : node_type == JFFS2_NODETYPE_DIRENT;
>              JFFS2_Sum_Inode inode : node_type == JFFS2_NODETYPE_INODE;
>              JFFS2_Sum_XAttr xattr : node_type == JFFS2_NODETYPE_XATTR;
>              JFFS2_Sum_XRef xref : node_type == JFFS2_NODETYPE_XREF;
>              JFFS2_Sum_Unk unknown;
>          } data @ 0#B;
>         };
>
>     This way you don't need a method at all :)

I'm thinking that we may be over-replicating the union logic here
unnecessarily.  Why not just turn JFFS2_Sum_Rec into an union:

type JFFS2_Sum_Rec =
  union
  {
     JFFS2_Sum_Dirent dirent;
     JFFS2_Sum_Inode inode;
     JFFS2_Sum_XAttr xattr;
     JFFS2_Sum_XRef xref;
     JFFS2_Sum_Unk unknown;

     method get_node_type = uint<16>:
     {
       try return dirent.node_type; catch if E_elem {}
       try return inode.node_type; catch if E_elem {}
       ...
       return unknown.node_type;
     }
  };

The method `get_node_type' may not be necessary at all, since the usual
way in poke to check whether a given union value is of some alternative
foo is to:

  try [.. do something with value.FOO ..]; catch if E_elem {...}

Yes, we probably want to add some syntax sugar for this idiom to the
language :)

Same for JFFS2_Node:

type JFFS2_Node =
  union
  {
    JFFS2_Dirent dirent;
    JFFS2_Inode inode;
    ...
    JFFS2_Unk_Node unknown;
  };