Hexadecimal to string without C++ standard library functions

Question

For the purposes of this code review, I will use the standard library, but pretend that <cstddef> is <stddef.h>, that I didn't use std::cout, that new and delete[] are overloaded and call custom malloc functions, etc. So this is -std=c++11 -ffreestanding.

Here are my questions:

• Currently my tohex function returns a buffer allocated on the heap. Is it better if I have the caller pass the buffer as a parameter instead? Is there a way to avoid heap allocation without using variable length arrays?
• I have a separate function that counts the size of the digits so that I can reverse the string. What's an alternate way of doing this?
• Should I prefer uint8_t or int or is unsigned okay? I don't think there should be any issues with what type I choose in the code shown.

#include <cstddef>

unsigned count(unsigned n)
{
    int digits = 0;
    while (n > 0)
    {
        ++digits;
        n /= 16;
    }
    return digits;
}

char* tohex(unsigned n)
{
    unsigned size = count(n) + 1;
    char *buffer = new char[size];
    unsigned index = size - 2;
    while (n > 0)
    {
        unsigned mod = n % 16;

        if (mod >= 10)
            buffer[index--] = (mod - 10) + 'A';
        else
            buffer[index--] = mod + '0';

        n /= 16;
    }
    buffer[size - 1] = '\0';
    return buffer;
}

#include <iostream>

int main()
{
    char *buffer = tohex(0xBEEFCAFE);
    std::cout << buffer;
    delete[] buffer;
    return 0;
}

Answer 1

As both the others have said use std::string if you are going to dynamically allocate space. Its not going to be worse than your code;if your compiler implements the small string optimizations its actually goinf to do better than your code as no dynamic memory will be used.

unsigned count(unsigned n)

Well we know that unsigned have a particular size sizeof(unsigned). So we know that the max size of the array is sizeof(unsigned)*CHAR_BITS/4. On most systems this will be 8 on some it will be 16. But neither of these numbers is huge why not always use the maximum size (it will save you the time of calculating an exact size.

You can also get away with no dynamic memory allocation by having a static array in the function (not thread safe but it does not look like that is your issue).

Others have suggested replacing the n /= 16; by n >> 4;. DON'T do that (unless you think it expresses intent better). It hides the intent of the code. The code is supposed to express the intent in the most natural way possible. It is the job of the compiler to see that this can be optimized to a shift by 4 and I am dead sure the compiler will generate the most efficient code.

Answer 2

• I see no reason to use char* over std::string here. The latter does the memory management for you (no need for new/delete). Whether or not this already leaks, using std::string should help eliminate this uncertainly.

• Keep your #includes at the very top, where they can be easily maintained. The compiler will still have to compile all the library code, so it doesn't really help to move any somewhere else.

• The name count() is misleading here as it just takes an int. Since it appears to return the number of digits in the argument, you should rename the function to digits().

Answer 3

For C++ it is always better to use class that manages the deallocation in destructor. This could be std::string or std::unique_ptr. For custom allocation/deallocation you should adapt class similar to std::allocator and use std::basic_string<char, std::char_traits<char>, my_allocator>. In case of std::unique_ptr, you can provide the Deleter. Ohter possibility would be to create your own wrapper similar to unique_ptr, but maybe with conversion operator const char* ().

You could pass the buffer into the function to make it thread-safe (not to return address of static internal buffer) and for this situation, I see no problem to have bigger buffer - it will still be quite small (9B).

---

Now few hints about optimization:

unsigned mod = n % 16;
n /= 16;

Division is slow and specifically these are better written as bit-masking and shifts:

unsigned mod = n & 15; // or: n & 0xF -- get/mask the nibble
n >>= 4;               // move to next nibble

---

unsigned vs. signed vs. other types: well, you better use unsigned not to run unto troubles with the signbit (sign-extension, sign-preserving shifts, division, modulo). But otherwise, it is your choice to use unsigned or uint32_t.

If the question was not about the argument (for tohex) but generic, then I would advise to use standard unsigned, int, char, unsigned char when used in the program, but uint8_t when you write some message-data-structure to be passed over IPC or packets (to make it architecture-independent). There are even big-endian and little-endian versions... but this seems to be far away from the simple example.