From: Christian Heller <c.heller@plomlompom.de>
Date: Thu, 15 Aug 2013 15:21:07 +0000 (+0200)
Subject: More comments on readwrite library; some refactoring ideas included.
X-Git-Tag: tce~1094
X-Git-Url: https://plomlompom.com/repos/%7B%7Bprefix%7D%7D/%7B%7Btodo.comment%7D%7D?a=commitdiff_plain;h=823357b90906df451402358eda7b995143922496;p=plomrogue

More comments on readwrite library; some refactoring ideas included.
---

diff --git a/src/readwrite.c b/src/readwrite.c
index 2b6823a..e776ec2 100644
--- a/src/readwrite.c
+++ b/src/readwrite.c
@@ -6,7 +6,15 @@
 
 
 
-/* Read/write "x" from/to "file" as bigendian representation of "size" bits. */
+/* Read/write "x" from/to "file" as bigendian representation of "size" bits.
+ * Only multiples of 8 allowed for "size". On failure, return 1, else 0.
+ *
+ * As of of now, all other read/write functions build on top of these. TODO:
+ * Consider externing these so-far internal functions and dropping the
+ * interfaces to them, instead relying on their internal validity checks.
+ * (Usage of padded memory copies instead of directly manipulating * x as is
+ * done in read_uint* would need to be added, though.)
+ */
 static uint8_t read_uintX_bigendian(FILE * file, uint32_t * x, uint8_t size);
 static uint8_t write_uintX_bigendian(FILE * file, uint32_t x, uint8_t size);
 
@@ -14,10 +22,10 @@ static uint8_t write_uintX_bigendian(FILE * file, uint32_t x, uint8_t size);
 
 static uint8_t read_uintX_bigendian(FILE * file, uint32_t * x, uint8_t size)
 {
-    if (0 != size % 8)
-    {
-        return 1;
-    }
+    if (0 != size % 8)           /* This bit number validity check is redundant */
+    {                            /* as long as this function is only available  */
+        return 1;                /* through extern interfaces pre-defining the  */
+    }                            /* bit number. TODO: Consider dropping it.     */
     int16_t bitshift = size - 8;
 
     * x = 0;
@@ -38,10 +46,10 @@ static uint8_t read_uintX_bigendian(FILE * file, uint32_t * x, uint8_t size)
 
 static uint8_t write_uintX_bigendian(FILE * file, uint32_t x, uint8_t size)
 {
-    if (0 != size % 8)
-    {
-        return 1;
-    }
+    if (0 != size % 8)                             /* See comment             */
+    {                                              /* on identical            */
+        return 1;                                  /* code block in           */
+    }                                              /* read_uintX_bigendian(). */
     int16_t bitshift = size - 8;
 
     for (; bitshift >= 0; bitshift = bitshift - 8)
@@ -58,10 +66,8 @@ static uint8_t write_uintX_bigendian(FILE * file, uint32_t x, uint8_t size)
 
 extern uint8_t read_uint8(FILE * file, uint8_t * x)
 {
-    /* Since read_uintX_bigendian() works on -- and zeroes -- four bytes, work
-     * on values of fewer bytes corrupts their immediate neighbor bytes. This
-     * necessitates working on newly acquired separate memory areas (* y), only
-     * copying the sufficiently small end result to * x.
+    /* Since read_uintX_bigendian() works on -- and zeroes -- four bytes, direct
+     * work on values of fewer bytes would corrupt immediate neighbor values.
      */
     uint32_t y = * x;
     uint8_t err = read_uintX_bigendian(file, &y, 8);