Re: [RFC] redoxfs.pk: Pickle for RedoxFS file system

[Jose E. Marchesi]

>> >     RedoxFS_Extent[(#RedoxFS_BLKSZ/#B - 288UL) / (#RedoxFS_Extent/#B)] 
>> > extents;
>> 
>> Hmm, how does the spec define the size of this array exactly?  Remember
>> Poke supports array types whose extension is defined by a size rather
>> than by number of elements... maybe it would be more clear to define it
>> that way?
>
>
> Sure!
> `RedoxFS_Extent[#RedoxFS_BLKSZ - 288#B]` is much more readable.
> BTW `RedoxFS_Extent[#RedoxFS_BLKSZ - OFFSET]` does not work as expected.
> Did I miss anything?

How it doesn't work as expected?

>From the manual:

"The offset of the current field being mapped or constructed, relative
 to the beginning of the struct, is at all times available in a variable
 called @code{OFFSET}.  This is useful in situations where we need that
 information in a constraint expression, the boundary of an array, an
 initialization value, or the like."

>> > fun redoxfs_nprint = (RedoxFS_Node n, int reverse = 0) void:
>> 
>> Wouldn't it be better to define this as a method in the RedoxFS_Node
>> type?
>
>
> It is better, but the `reverse` argument is a blocker (I think I can
> get rid of it, `reverse` is only useful when all `extents` including the
> empty ones get printed).

Well, but as I suggested, if you install pretty-printers in the
RedoxFS_Extent type, then the user can very easily print it reversed.

I am thinking, maybe we should allow _print methods with additional
optional arguments, like:

 method _print = (int reverse_p = 0) void: { ... }

The additional arguments would need to be optional, but maybe this is
useful enough?

>> >
>> > type RedoxFS_EVisitor = (RedoxFS_Extent) void;
>> > fun redoxfs_extents = (RedoxFS_Node n, RedoxFS_EVisitor v) uint<64>:
>> >   {
>> 
>> Wouldn't it be better to define this as a method in the RedoxFS_Node
>> type?
>
>
> I cannot. Because of this line `n = RedoxFS_Node @ nxt;` below:

Hmm, I see now the comment:

  /* Visits all the extents of a node (extents + next.extents +
   * next.next.extents + ...).
   */

Then I wonder, what about having a method visit_extents in RedoxFS_Node
that will handle the extents of that method:

  method visit_extents = (RedoxFS_EVisitor v) uint64:
  {
    var counter = 0;

    for (e in n.extents where !e.empty)
      {
        counter += 1;
        v (e);
      }

     return counter;
  }

And then a method visit_extents in the data structure that contains the
root node of the filesystem, or has the information to find it?

For example, the header:

type RedoxFS_Header =
  struct
  {
    [...]
    offset<uint<64>,RedoxFS_BLKSZ> root;  /* Block of root node */
    [...]

    method visit_extents (RedoxFS_EVisitor v) uint64:
    {
     var counter = 0;
     var next = node.next;

     while (next != 0#B)
     {
       node = RedoxFS_Node @ root;
       next = node.next;
       counter += node.visit_extents (v);
     }

     return counter;
    }
   }

But it would probably be a better abstraction too add a
RedoxFS_Filesystem like this:

type RedoxFS_Filesystem =
  struct
  {
    RedoxFS_Header header;
    RedoxFS_Node root @ header.root;
    ...

    method visit_extents (RedoxFS_EVisitor v) uint64:
    {
      /* Like above but using root instead of RedoxFS_Node @ root. */
    }
  }

WDYT?

>> >     while (nxt != 0#B)
>> >       {
>> >         n = RedoxFS_Node @ nxt;
>
> Here.
>
>> >
>> > fun redoxfs_walk = (RedoxFS_Node n, RedoxFS_Visitor v) void:
>> 
>> Wouldn't it be better to define this as a method in the RedoxFS_Node
>> type?
>> 
>
> Ditto.

See above.