On 02/19/2018 11:00 AM, Thomas Huth wrote:
On 19.02.2018 16:40, Collin L. Walling wrote:
On 02/17/2018 02:48 AM, Thomas Huth wrote:
On 16.02.2018 23:07, Collin L. Walling wrote:
+ * uitoa:
+ * @num: an integer (base 10) to be converted.
+ * @str: a pointer to a string to store the conversion.
+ * @len: the length of the passed string.
+ *
+ * Given an integer @num, convert it to a string. The string @str
must be
+ * allocated beforehand. The resulting string will be null
terminated and
+ * returned. This function only handles numbers between 0 and
+ * inclusive.
+ *
+ * Returns: the string @str of the converted integer @num
+ */
+char *uitoa(uint64_t num, char *str, size_t len)
+    size_t num_idx = 0;
+    uint64_t tmp = num;
+    IPL_assert(str != NULL, "uitoa: no space allocated to store
+    /* Get index to ones place */
+    while ((tmp /= 10) != 0) {
+        num_idx++;
+    }
+    /* Check if we have enough space for num and null */
+    IPL_assert(len > num_idx, "uitoa: array too small for conversion");
Well, in v5 of this patch you've had "len >= num_idx + 1" where we
agreed that it was wrong. Now you have "len > num_idx" which is pretty
much the same. WTF?
I still think you need "len > num_idx + 1" here to properly take the
trailing NUL-byte into account properly. Please fix it!


You're correct, and my apologies for not correcting the true problem here:
I messed up the value of num_idx.  It is off by one, but initializing the
value to 1 instead of 0 should fix this. I must've accounted for this in
my test file but forgot to update it in the actual source code.
Are you sure that initializing it to 1 is right? Unless you also change
the final while loop in this function, this will put the character into
the wrong location ("str[num_idx] = num % 10 + '0';"). Imagine a number
that only consists of one digit ... the digit will be placed in str[1]
which sounds wrong to me...?


There's that, which we can solve by decrementing num_idx at the start of the loop.
We also have to change the line str[num_idx + 1] = '\0'; to no longer add 1.
It all boils down to "which way reads better", which I often struggle and
bounce back-and-forth with (way) too much...

Maybe I should also rename num_idx to just "idx" and let the comments explain

- Collin L Walling

Reply via email to