From 01d1db661c0743a54e9c28ef39ef02ecad82fe69 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Reynir=20Bj=C3=B6rnsson?= <reynir@reynir.dk>
Date: Wed, 15 Dec 2021 16:12:31 +0100
Subject: [PATCH] Remove inline record, documentation

The single-field records are unnecessary and a left-over from an earlier
iteration of the code.
---
 lib/solo5_elftool.ml  | 24 +++++++++++++-----------
 lib/solo5_elftool.mli | 16 ++++++++++------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/lib/solo5_elftool.ml b/lib/solo5_elftool.ml
index 197c837..9d27ec8 100644
--- a/lib/solo5_elftool.ml
+++ b/lib/solo5_elftool.ml
@@ -4,12 +4,8 @@ type mft_type =
   | Reserved_first
 
 type mft_entry =
-  | Dev_block_basic of {
-      name : string;
-    }
-  | Dev_net_basic of {
-      name : string;
-    }
+  | Dev_block_basic of string
+  | Dev_net_basic of string
 
 type mft = {
   version : int;
@@ -23,9 +19,9 @@ let mft_type_of_int : int32 -> mft_type = function
   | _ -> assert false
 
 let pp_mft_entry ppf = function
-  | Dev_block_basic { name; _ } ->
+  | Dev_block_basic name ->
     Fmt.pf ppf {|{@[<1>@ "name": %S,@ "type": "BLOCK_BASIC"@]@ }|} name
-  | Dev_net_basic { name; _ } ->
+  | Dev_net_basic name ->
     Fmt.pf ppf {|{@[<1>@ "name": %S,@ "type": "NET_BASIC"@]@ }|} name
 
 let pp_mft ppf { version; entries } =
@@ -68,7 +64,10 @@ let parse_mft buf =
   let* () = guard "manifest too small"
       (Cstruct.length buf >= 4 + 8 + sizeof_mft_entry)
   in
-  (* FIXME: explanation why solo5 adds this padding *)
+  (* Solo5 defines a struct mft1_note consisting of the ELF note header
+   * followed by a struct mft for reading and writing the ELF note. The note
+   * header is 20 bytes long, so to get 8-byte alignment the note header is
+   * padded with 4 bytes. See {[solo5/mft_abi.h]}. *)
   let buf = Cstruct.shift buf 4 in
   let version = Cstruct.LE.get_uint32 buf 0
   and entries = Cstruct.LE.get_uint32 buf 4
@@ -79,6 +78,9 @@ let parse_mft buf =
   let* () = guard "too many manifest entries"
       (Int32.unsigned_compare entries mft_max_entries <= 0)
   in
+  (* We have checked that entries interpreted unsigned is between 0 and
+   * mft_max_entries, so this is safely equivalent to:
+   *   (Option.get (Int32.unsigned_to_int entries) *)
   let entries = Int32.to_int entries in
   let buf = Cstruct.shift buf 8 in
   let* () = guard "unexpected note size"
@@ -100,8 +102,8 @@ let parse_mft buf =
          let* acc = r in
          let* mft_entry = parse_mft_entry buf in
          match mft_entry with
-         | `Dev_block_basic name -> Ok (Dev_block_basic { name } :: acc)
-         | `Dev_net_basic name -> Ok (Dev_net_basic { name } :: acc)
+         | `Dev_block_basic name -> Ok (Dev_block_basic name :: acc)
+         | `Dev_net_basic name -> Ok (Dev_net_basic name :: acc)
          | `Reserved_first -> Error (`Msg "found RESERVED_FIRST not as first entry"))
       (Ok [])
       entries
diff --git a/lib/solo5_elftool.mli b/lib/solo5_elftool.mli
index dfc4d50..c3e5712 100644
--- a/lib/solo5_elftool.mli
+++ b/lib/solo5_elftool.mli
@@ -1,17 +1,21 @@
+(** An entry in the manifest representing a device. *)
 type mft_entry =
-  | Dev_block_basic of {
-      name : string;
-    }
-  | Dev_net_basic of {
-      name : string;
-    }
+  | Dev_block_basic of string
+  | Dev_net_basic of string
 
+(** The Solo5 manifest *)
 type mft = {
   version : int;
+  (** [version] is at the moment always 1. *)
   entries : mft_entry list;
+  (** [entries] in the manifest. *)
 }
 
 val pp_mft_entry : Format.formatter -> mft_entry -> unit
 val pp_mft : Format.formatter -> mft -> unit
+(** Pretty-prints the manifest as JSON in a similar style as the Solo5 command
+ * line tool {[solo5-elftool query-manifest]}. *)
 
 val query_manifest : Owee_buf.t -> (mft, [> `Msg of string ]) result
+(** [query_manifest buf] is the solo5 manifest of [buf], or an error message.
+ * @raise Owee_buf.Invalid_format If [buf] does not contain valid ELF format *)