Moved:
  memcmp from bootmap.h to libc.h (renamed from _memcmp)
  strlen from sclp.c to libc.h (renamed from _strlen)

Added C standard functions:
  isdigit

Added non C-standard function:
  uitoa
  atoui

Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
 pc-bios/s390-ccw/Makefile  |  2 +-
 pc-bios/s390-ccw/bootmap.c |  4 +--
 pc-bios/s390-ccw/bootmap.h | 16 +--------
 pc-bios/s390-ccw/libc.c    | 89 ++++++++++++++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/libc.h    | 37 +++++++++++++++++--
 pc-bios/s390-ccw/main.c    | 17 +--------
 pc-bios/s390-ccw/sclp.c    | 10 +-----
 7 files changed, 130 insertions(+), 45 deletions(-)
 create mode 100644 pc-bios/s390-ccw/libc.c

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 6d0c2ee..9f7904f 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
 
 .PHONY : all clean build-all
 
-OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o
+OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
 QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
 QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index a94638d..092fb35 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -506,7 +506,7 @@ static bool is_iso_bc_entry_compatible(IsoBcSection *s)
                     "Failed to read image sector 0");
 
     /* Checking bytes 8 - 32 for S390 Linux magic */
-    return !_memcmp(magic_sec + 8, linux_s390_magic, 24);
+    return !memcmp(magic_sec + 8, linux_s390_magic, 24);
 }
 
 /* Location of the current sector of the directory */
@@ -635,7 +635,7 @@ static uint32_t find_iso_bc(void)
         if (vd->type == VOL_DESC_TYPE_BOOT) {
             IsoVdElTorito *et = &vd->vd.boot;
 
-            if (!_memcmp(&et->el_torito[0], el_torito_magic, 32)) {
+            if (!memcmp(&et->el_torito[0], el_torito_magic, 32)) {
                 return bswap32(et->bc_offset);
             }
         }
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 4bd95cd..4cf7e1e 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -328,20 +328,6 @@ static inline bool magic_match(const void *data, const void *magic)
     return *((uint32_t *)data) == *((uint32_t *)magic);
 }
 
-static inline int _memcmp(const void *s1, const void *s2, size_t n)
-{
-    int i;
-    const uint8_t *p1 = s1, *p2 = s2;
-
-    for (i = 0; i < n; i++) {
-        if (p1[i] != p2[i]) {
-            return p1[i] > p2[i] ? 1 : -1;
-        }
-    }
-
-    return 0;
-}
-
 static inline uint32_t iso_733_to_u32(uint64_t x)
 {
     return (uint32_t)x;
@@ -434,7 +420,7 @@ const uint8_t vol_desc_magic[] = "CD001";
 
 static inline bool is_iso_vd_valid(IsoVolDesc *vd)
 {
-    return !_memcmp(&vd->ident[0], vol_desc_magic, 5) &&
+    return !memcmp(&vd->ident[0], vol_desc_magic, 5) &&
            vd->version == 0x1 &&
            vd->type <= VOL_DESC_TYPE_PARTITION;
 }
diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c
new file mode 100644
index 0000000..a144388
--- /dev/null
+++ b/pc-bios/s390-ccw/libc.c
@@ -0,0 +1,89 @@
+/*
+ * libc-style definitions and functions
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Collin L. Walling <walling@linux.vnet.ibm.com>
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+
+/**
+ * atoui:
+ * @str: the string to be converted.
+ *
+ * Given a string @str, convert it to an integer. Leading spaces are
+ * ignored. Any other non-numerical value will terminate the conversion
+ * and return 0. This function only handles numbers between 0 and
+ * UINT64_MAX inclusive.
+ *
+ * Returns: an integer converted from the string @str, or the number 0
+ * if an error occurred.
+ */
+uint64_t atoui(const char *str)
+{
+    int val = 0;
+
+    if (!str || !str[0]) {
+        return 0;
+    }
+
+    while (*str == ' ') {
+        str++;
+    }
+
+    while (*str) {
+        if (!isdigit(*str)) {
+            break;
+        }
+        val = val * 10 + *str - '0';
+        str++;
+    }
+
+    return val;
+}
+
+/**
+ * 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 UINT64_MAX
+ * 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 string");
+
+    /* 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");
+
+    str[num_idx + 1] = '\0';
+
+    /* Convert int to string */
+    while (num_idx >= 0) {
+        str[num_idx] = num % 10 + '0';
+        num /= 10;
+        num_idx--;
+    }
+
+    return str;
+}
diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
index 0142ea8..63ece70 100644
--- a/pc-bios/s390-ccw/libc.h
+++ b/pc-bios/s390-ccw/libc.h
@@ -1,6 +1,8 @@
 /*
  * libc-style definitions and functions
  *
+ * Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ *
  * This code is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
  * Free Software Foundation; either version 2 of the License, or (at your
@@ -19,7 +21,7 @@ typedef unsigned long long uint64_t;
 
 static inline void *memset(void *s, int c, size_t n)
 {
-    int i;
+    size_t i;
     unsigned char *p = s;
 
     for (i = 0; i < n; i++) {
@@ -33,7 +35,7 @@ static inline void *memcpy(void *s1, const void *s2, size_t n)
 {
     uint8_t *dest = s1;
     const uint8_t *src = s2;
-    int i;
+    size_t i;
 
     for (i = 0; i < n; i++) {
         dest[i] = src[i];
@@ -42,4 +44,35 @@ static inline void *memcpy(void *s1, const void *s2, size_t n)
     return s1;
 }
 
+static inline int memcmp(const void *s1, const void *s2, size_t n)
+{
+    size_t i;
+    const uint8_t *p1 = s1, *p2 = s2;
+
+    for (i = 0; i < n; i++) {
+        if (p1[i] != p2[i]) {
+            return p1[i] > p2[i] ? 1 : -1;
+        }
+    }
+
+    return 0;
+}
+
+static inline size_t strlen(const char *str)
+{
+    size_t i;
+    for (i = 0; *str; i++) {
+        str++;
+    }
+    return i;
+}
+
+static inline int isdigit(int c)
+{
+    return (c >= '0') && (c <= '9');
+}
+
+uint64_t atoui(const char *str);
+char *uitoa(uint64_t num, char *str, size_t len);
+
 #endif
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 401e9db..e857ce4 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -40,22 +40,7 @@ void panic(const char *string)
 
 unsigned int get_loadparm_index(void)
 {
-    const char *lp = loadparm;
-    int i;
-    unsigned int idx = 0;
-
-    for (i = 0; i < 8; i++) {
-        char c = lp[i];
-
-        if (c < '0' || c > '9') {
-            break;
-        }
-
-        idx *= 10;
-        idx += c - '0';
-    }
-
-    return idx;
+    return atoui(loadparm);
 }
 
 static bool find_dev(Schib *schib, int dev_no)
diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
index 90d1bc3..e6a0898 100644
--- a/pc-bios/s390-ccw/sclp.c
+++ b/pc-bios/s390-ccw/sclp.c
@@ -65,14 +65,6 @@ void sclp_setup(void)
     sclp_set_write_mask();
 }
 
-static int _strlen(const char *str)
-{
-    int i;
-    for (i = 0; *str; i++)
-        str++;
-    return i;
-}
-
 long write(int fd, const void *str, size_t len)
 {
     WriteEventData *sccb = (void *)_sccb;
@@ -113,7 +105,7 @@ long write(int fd, const void *str, size_t len)
 
 void sclp_print(const char *str)
 {
-    write(1, str, _strlen(str));
+    write(1, str, strlen(str));
 }
 
 void sclp_get_loadparm_ascii(char *loadparm)
-- 
2.7.4

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 UINT64_MAX
> + * 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 string");
> +
> +    /* 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!

 Thomas

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 UINT64_MAX
>> + * 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 string");
>> +
>> +    /* 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!
>
>   Thomas
>
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.

I think the assertion make more sense as len > num_idx, if num_idx were
actually correct. Sorry for this mess...



-- 
- Collin L Walling

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
>>> UINT64_MAX
>>> + * 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
>>> string");
>>> +
>>> +    /* 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!
>>
>>   Thomas
>>
> 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...?

 Thomas

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
>>>> UINT64_MAX
>>>> + * 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
>>>> string");
>>>> +
>>>> +    /* 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!
>>>
>>>    Thomas
>>>
>> 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...?
>
>   Thomas
>
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
everything?

-- 
- Collin L Walling

On 02/19/2018 11:19 AM, Collin L. Walling wrote:
> 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
>>>>> UINT64_MAX
>>>>> + * 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
>>>>> string");
>>>>> +
>>>>> +    /* 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!
>>>>
>>>>    Thomas
>>>>
>>> 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...?
>>
>>   Thomas
>>
> 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
> everything?
>
How is this for a compromise?

     - start num_idx at 1, provide comment as for why
     - change while loop comment to explain we are "counting the 
_indices_ _of_ _num_"
     - str[num_idx] is assigned \0, and we also decrement num_idx in one 
line
     - in conversion loop, post decrement num_idx as it is used

char *uitoa(int num, char *str, int len)
{
   int num_idx = 1; /* account for NULL */
   int tmp = num;

   assert(str != NULL, "uitoa: no space allocated to store string");

   /* Count indices of num */
   while ((tmp /= 10) != 0)
     num_idx++;

   /* Check if we have enough space for num and null */
   assert(len > num_idx, "uitoa: array too small for conversion");

   str[num_idx--] = '\0';

   /* Convert int to string */
   while (num_idx >= 0) {
     str[num_idx--] = num % 10 + '0';
     num /= 10;
   }

   return str;
}


-- 
- Collin L Walling

On 02/19/2018 11:17 AM, Collin L. Walling wrote:

> How is this for a compromise?
> 
>      - start num_idx at 1, provide comment as for why
>      - change while loop comment to explain we are "counting the 
> _indices_ _of_ _num_"
>      - str[num_idx] is assigned \0, and we also decrement num_idx in one 
> line
>      - in conversion loop, post decrement num_idx as it is used
> 
> char *uitoa(int num, char *str, int len)
> {
>    int num_idx = 1; /* account for NULL */

The single-byte character is named NUL (while NULL refers to the 8- or 
4-byte pointer value).

>    int tmp = num;
> 
>    assert(str != NULL, "uitoa: no space allocated to store string");
> 
>    /* Count indices of num */
>    while ((tmp /= 10) != 0)
>      num_idx++;
> 
>    /* Check if we have enough space for num and null */

and again

>    assert(len > num_idx, "uitoa: array too small for conversion");
> 
>    str[num_idx--] = '\0';
> 
>    /* Convert int to string */
>    while (num_idx >= 0) {
>      str[num_idx--] = num % 10 + '0';
>      num /= 10;
>    }
> 
>    return str;

Otherwise, it seems readable to me.

> }
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 19.02.2018 18:17, Collin L. Walling wrote:
> On 02/19/2018 11:19 AM, Collin L. Walling wrote:
>> 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
>>>>>> UINT64_MAX
>>>>>> + * 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
>>>>>> string");
>>>>>> +
>>>>>> +    /* 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!
>>>>>
>>>>>    Thomas
>>>>>
>>>> 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...?
>>>
>>>   Thomas
>>>
>> 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
>> everything?
>>
> How is this for a compromise?
> 
>     - start num_idx at 1, provide comment as for why
>     - change while loop comment to explain we are "counting the
> _indices_ _of_ _num_"
>     - str[num_idx] is assigned \0, and we also decrement num_idx in one
> line
>     - in conversion loop, post decrement num_idx as it is used
> 
> char *uitoa(int num, char *str, int len)
> {
>   int num_idx = 1; /* account for NULL */
>   int tmp = num;
> 
>   assert(str != NULL, "uitoa: no space allocated to store string");
> 
>   /* Count indices of num */
>   while ((tmp /= 10) != 0)
>     num_idx++;
> 
>   /* Check if we have enough space for num and null */
>   assert(len > num_idx, "uitoa: array too small for conversion");
> 
>   str[num_idx--] = '\0';
> 
>   /* Convert int to string */
>   while (num_idx >= 0) {
>     str[num_idx--] = num % 10 + '0';
>     num /= 10;
>   }
> 
>   return str;
> }

Yes, looks fine to me that way (with the "NUL" change mentioned by Eric).

 Thomas