What's wrong w/my code? Maybe I don't know snprintf()

I'm trying to write an optimal function that converts a blob into a hexadecimal character string. It's written in C++, but I think it is also C

Code:

---

        const unsigned char *pBytes = (const unsigned char *)p;
        char *pOut = (char *)malloc(nLength*2 + 1);
        for(size_t i = 0; i < nLength; i++) {
                unsigned int byte = (unsigned int)pBytes[i];
                assert(byte <= 0xFF);
                verify(snprintf(&pOut[2*i], 2, "%02X", byte) == 2);
        }
        pOut[nLength*2] = '\0';
fprintf(stderr, "strlen was:  %d and nlegnth*2: %d\n", (int)strlen(pOut), (int)2*nLength));
        assert(strlen(pOut) == nLength*2);

---

That little debug output reveals that strlen is 1 while nLength*2 is 96. My best guess is that snprintf() doesn't work the way I think it's supposed to.
Obviously, the program ends with the assert in the last line.

Or the second byte is 0x00?

What is verify? What's it supposed to do?

I think your problem is that you pass a length of 2 to snprintf. So the buffer has only 2 characters, according to sprintf. However, it tries to write 3: "XX\0". So it won't return 2, but 1, as only 1 byte excluding the null-character has been written.

Also, why use snprintf if you know your buffer is large enough?

Finally, always check the return value of malloc... Make sure it's not NULL.

Quote:

---

Originally Posted by EVOEx

What is verify? What's it supposed to do?

---

That's borrowed from Visual Studio's VERIFY(). As far as this code is concerned, it is the same as assert().

Quote:

---

think your problem is that you pass a length of 2 to snprintf. So the buffer has only 2 characters, according to sprintf. However, it tries to write 3: "XX\0". So it won't return 2, but 1, as only 1 byte excluding the null-character has been written.

---

If snprintf() returned anything but 2 then verify() would have aborted the program. And it returns 2 because that's the number of characters that there are in a hexadecimal string representing 1 byte.

Quote:

---

Also, why use snprintf if you know your buffer is large enough?

---

It was my thinking that the code would be optimal if it didn't constantly write out the null terminator. As you can see, the null terminator would be unnecessarily written with every iteration. The only iteration where it would be necessary would be the last one.

Quote:

---

Finally, always check the return value of malloc... Make sure it's not NULL.

---

I will, but if that was the problem I would have gotten a seg fault, and I'm running this through valgrind, which hasn't reported any problems.

Quote:

---

Originally Posted by man snprintf

The functions snprintf() and vsnprintf() do not write more than size bytes (including the trailing '\0'). If the output was trun-
cated due to this limit then the return value is the number of characters (not including the trailing '\0') which would have been
written to the final string if enough space had been available. Thus, a return value of size or more means that the output was
truncated. (See also below under NOTES.)

---

From this text, I'd guess that the reason that you get a 1 in place of a 2 is that the string was truncated. Looks like the '\0' will be written regardless of whether your text is.

If you're so concerned about efficiency, just write it yourself . . . . e.g.

Code:

---

#include <stdio.h>
#include <stdlib.h>

char to_digit(int hex_digit) {
    if(hex_digit < 10) return hex_digit + '0';
   
    return (hex_digit - 10) + 'a';
}

char *to_hex_string(const unsigned char *data, size_t length) {
    size_t n = 0;
    char *p = malloc(length * 2 + 1);
   
    for(n = 0; n < length; n ++) {
        p[n * 2 + 0] = to_digit(data[n] >> 4);
        p[n * 2 + 1] = to_digit(data[n] & 0xf);
    }
   
    p[length * 2] = 0;
   
    return p;
}

int main() {
    unsigned char data[] = {0x32, 0x20, 0xfa, 0xff};
    char *str = to_hex_string(data, sizeof data / sizeof *data);
   
    puts(str);
    free(str);
   
    return 0;
}

---

I'm sorry, brain........ of me there ;). Well, like I said, it does write one character and then a 0-terminating character, as you only have 2 bytes. snprintf returns how many bytes it WOULD have written, would it have enough space, which is 2 bytes (+1 0-byte). But it only wrote 1.
Which is also why you get a strlen of 1.

Also, this is bad optimization. sprintf writes a 0-byte every time but would still be faster.

I think that the last 3 posts are correct. I suppose what the man pages should say is that if the output gets truncated then you cannot trust what the output is, and should redo the function again with a buffer big enough for so as not to cause truncation.
Thanks guys.