0

I need to send some unsigned longs via sockets. Due to the fact an unsigned long is 4 Byte, the receiver expects only 4 Byte. The conversion function I wrote and you will find below works, but only if the number that has to be stored in the char is not bigger then 127 resp. 0x7F. For values bigger 0x7f I would expect characters according the extended ASCII table (http://www.asciitable.com/) are going to be stored in the char, but that's definitely not he case. For 0x90 for example nothing is stored. I'm using VS12 with Unicode character set.

Any idea's how to make the conversion correct?

void number2char(unsigned long number, char* nrAsByte){
    std::stringstream numberSS;
    numberSS << std::hex << number;
    int length = numberSS.str().length();
    length = length / 2.0 + 0.5;
    nrAsByte = new char[sizeof(number)]();
    std::fill(nrAsByte, nrAsByte + length, '\x20');
    while (length > 0){
        int lastTwo = (number & 0xff);
        number >>= 8;
        unsigned char a = lastTwo;  // this doesn't work if lastTwo > 0x7F
        std::memcpy(nrAsByte + length - 1, &a, 1);
        --length;
    }
}

I'm sorry for the code, it wasn't well tested by me and it's containing bugs, please DON'T USE IT, follow the advices in the answers instead

GregPhil
  • 475
  • 1
  • 8
  • 20
  • 5
    Why not sending the 4 bytes directly? – Mine Jul 13 '16 at 07:50
  • In what way doesn't it work? – user253751 Jul 13 '16 at 07:50
  • The function you posted leaks memory. Memory is allocated for `nrAsByte`, but no mechanism for the caller to get a pointer to the memory is provided. Basically you seem to try to do the conversion, but the result is not transferred to the caller... – skyking Jul 13 '16 at 07:51
  • @skyking sorry I didn't check for memory leaks yet. – GregPhil Jul 13 '16 at 07:54
  • @GregPhil But that's not the only problem. Your function basically keeps the result for itself. – skyking Jul 13 '16 at 07:55
  • You can just get the address of number and copy four bytes after that address to `char` array. Do you want to do something different? –  Jul 13 '16 at 07:57
  • @skyking you are right. I had to check if the caller get the result as well. I was checking only the conversion itself. Shame on me. – GregPhil Jul 13 '16 at 07:58
  • Also your way of allocatidng `nrAsByte` and determining the length may be inconsistent. What happens if `length > sizeof(number)`? Then you would have a buffer overrun when doing `memcpy`. – skyking Jul 13 '16 at 07:59
  • 1
    @Mine, because you aren't guaranteed unsigned long is 4 bytes on source system. So you need to send `sizeof(unsigned long)` bytes. – StoryTeller - Unslander Monica Jul 13 '16 at 08:00
  • Also filling `nrAsByte` with `'\0x20'` makes one wonder what you're trying to do. Do you want to store `number` in hexadecimal form or binary? If it's the later filling with `\0x20` is probably a bad/confusing idea. – skyking Jul 13 '16 at 08:02
  • 2
    You should normalize the endianess using `htonl()`, and then simply cast the address of the result to `char*` for sending. – πάντα ῥεῖ Jul 13 '16 at 08:02
  • if the target would be char[4], on platforms having 32b unsigned and correct endian you can do just return `(char *)(&number);` If the target is fake "string" (`char *`), you need zero terminator after number, so you should allocate somewhere 5 bytes, copy the number into first 4 and add 0 at end. I don't see how this even touch encoding (ascii/unicode), as you can send binary data directly (endianness solved with `htonl`). If you really want to convert number to string and back, characters [0-9a-f] are ASCII, so 4 chars should handle anything from 0x0000 to 0xFFFF (16b integer). – Ped7g Jul 13 '16 at 08:12
  • http://stackoverflow.com/a/1703521/4944425 – Bob__ Jul 13 '16 at 08:12
  • Oh, I was asleep, when I was adding my comment. If you want zero-terminated string, you can't copy binary unsigned into it, because it may contain some zero byte itself, like `0x12003456`, which would terminate the string prematurely. So obviously the char[4] (ie. enforced 4 byte long transmission) is required for binary form of transmission. – Ped7g Jul 13 '16 at 10:57

2 Answers2

2

Why not something like:

void number2char(unsigned long number, char* nrAsByte){
   unsigned char *dst= reinterpret_cast<unsigned char *> nrAsByte;
   for (int i=0; i<sizeof(unsigned long); ++i) {
      *dst++= number & 0xFF;
      number >>= 8;
   }
}
EFenix
  • 831
  • 4
  • 11
1

Hm, I've been toying around with Antonio's answer, as it didn't feel complete and correct, and in the end I ended with something more complex than I expected, but complexity has it's purpose sometimes.

Following code is doing among other things also htonl/ntohl-like conversion manually (maybe in opposite endianness, so it's not desirable to mix this with htonl, use either this, or rewrite it with htonl).

Unlike Antonio's source, it will not overwrite memory when input number type is 8 bytes long (unsigned long is 8 bytes on my test platform - BTW, try it?!), it will instead truncate the value to fit into the desired network char* buffer.

I tried to comment it extensively to give you idea behind each decision to add further complexity (to what is basically using unsigned int number used as (char *)(&number), which delivers too, but doesn't guard endianness, and may end in overwriting memory if you mix up different length types). But ask anything if you see something unclear.

#include <iostream>
#include <string>

// Experiment with different types to see differences
// (and how data are truncated when sizeof number > sizeof networkdata)

//typedef unsigned int numberType_t;
typedef unsigned long numberType_t;  // on my platform this is 8 bytes long

constexpr int networkBytesSize = 4;  // number of chars to be sent trough network with (char *)
// define network data type:
// used "struct" to make sizeof(networkData_t) return actual number of bytes
typedef struct {
    unsigned char d[networkBytesSize];
    char *cptr() { return reinterpret_cast<char *>(d); }
} networkData_t;

// Writes number into network char* buffer nrAsByte, endianness agnostic
void number2char(numberType_t number, networkData_t & nrAsByte) {
    for (size_t i = 0; i < sizeof(networkData_t); ++i) {
        nrAsByte.d[i] = number & 0xFF;
        number >>= 8;
    }
}

// Read number back from network char* buffer
numberType_t char2number(const networkData_t & nrAsByte) {
    numberType_t number = 0;
    size_t i = sizeof(networkData_t);
    while (i--) number = (number<<8) | nrAsByte.d[i];
    return number;
}

int main()
{
    printf("numberType_t size in bytes: %lu, networkData_t size in bytes: %lu\nAll following numbers are hex:\n",
            sizeof(numberType_t), sizeof(networkData_t));

    numberType_t number = numberType_t(0x9ABCDEF0123456FEul);
    std::cout << "source number: " << std::hex << number << std::endl;

    // Write number into char buffer
    networkData_t networkData;
    number2char(number, networkData);
    std::cout << "network bytes:";
    for (size_t i = 0; i < sizeof(networkData_t); ++i) std::cout << " [" << unsigned(networkData.d[i]) << "]";
    std::cout << std::endl;

    // Test usability of (char *) pointer access
    const char * testCharPtrConversion = networkData.cptr();
    printf("as char * (decimal signed): %d %d ...\n", testCharPtrConversion[0], testCharPtrConversion[1]);

    // Read number from char buffer
    number = char2number(networkData);
    std::cout << "read number: 0x" << std::hex << number << std::endl;
}
Ped7g
  • 16,236
  • 3
  • 26
  • 63
  • and maybe I should have show how to convert `char *` into networkData_t... turned out it will be probably most easy and clean to change networkData_t into ordinary C++ struct with two constructors: http://cpp.sh/3gvo (sort of worrying how many mistakes I made in such short time, I would strongly advice to build full class with all these together, and write thorough unit tests to validate everything works as expected). – Ped7g Jul 13 '16 at 12:38
  • 1
    AFAIK htonl converts to Big Endian, while your code converts (only the 4 least significant octets) to little endian. – Bob__ Jul 13 '16 at 14:54
  • @Bob__ Well, I meant it as "similar to htonl", didn't care if it's exact, so you may be correct, thank you for catching it. I will edit my wording to indicate, that it is not an replacement, but alternative (to show, how the endianness may be resolved on both ends). – Ped7g Jul 13 '16 at 15:01