C++ BitStreaming Decoding Function Returning Poor Values (Pointer Problems?)

My function READ(), seems to be working correctly, except when I dump the inputs I am getting more than what I asked for. Weird chars that are not 0s nor 1s. I bet it is due to my pointer usage (dereference operators), but I just cant seem to figure out what is doing it. Any help on making it output the correct data would be helpful. Also, some explanation why its wrong usage, and some good tips on how to never do it again. Thanks, you guys are the best.

Compiling under MSVC++ 2010 Express:

#include <iostream> 
using namespace std;

struct Holder
{
    unsigned char *bits;        // holds our bits
    unsigned char header[6];    // Always 6 bits
    unsigned char hasid[1];     // Always 1 bit
    unsigned char cid[4];       // always 4 bits
};

// Turns our Bytes Into Bits
// Credit: Ben Voigt(http://stackoverflow.com)
void BIT(const char *bytes, size_t len, char *bitStr)
{
    while (len--) {
        bitStr[0] = (*bytes & 0x80) ? '1': '0';
        bitStr[1] = (*bytes & 0x40) ? '1': '0';
        bitStr[2] = (*bytes & 0x20) ? '1': '0';
        bitStr[3] = (*bytes & 0x10) ? '1': '0';
        bitStr[4] = (*bytes & 0x08) ? '1': '0';
        bitStr[5] = (*bytes & 0x04) ? '1': '0';
        bitStr[6] = (*bytes & 0x02) ? '1': '0';
        bitStr[7] = (*bytes & 0x01) ? '1': '0';
        bitStr += 8;
        bytes++;
    }

    *bitStr = 0;
}

// This function is not working correctly
void READ( unsigned char *BITS, unsigned char *PACKET_ID, unsigned char *HAS_CHANNEL_ID, unsigned char *CHANNEL_ID, unsigned char *PACKET)
{
//=====================================================
// The Below Header Info Is Always The Same
//=====================================================
// Packet ID ALWAYS 6 BITS
    PACKET_ID[0] = BITS[2];
    PACKET_ID[1] = BITS[3];
    PACKET_ID[2] = BITS[4];
    PACKET_ID[3] = BITS[5];
    PACKET_ID[4] = BITS[6];
    PACKET_ID[5] = BITS[7];

// Packet Has Channel ID (1==true) ALWAYS 1 BIT
    HAS_CHANNEL_ID[0] = BITS[1];

// Channel ID ALWAYS 4 BIT
    CHANNEL_ID[0] = BITS[0];
    CHANNEL_ID[1] = BITS[15];
    CHANNEL_ID[2] = BITS[14];
    CHANNEL_ID[3] = BITS[13];
//=====================================================
// The above Header Info Is Always The Same
//=====================================================
//Variables For Looping
int P = 16; // This is the start of all data, always
int B = 24; // Every 24 bits, the loop goes into an IF_STATMENT and reads backwards into the array, then continues normally
int A = 1;  // This Increases every 24 bits
int Z = 0;  // This holds how many times we loop.  Used for IF_STATMENT formula
int I = 0;  // This gets zeroed out every IF_STATMENT and keeps our loop int i in sync
        PACKET[0] = BITS[12];
        PACKET[1] = BITS[11];
        PACKET[2] = BITS[10];
        PACKET[3] = BITS[9];
        PACKET[4] = BITS[8];

// Starts our main loop...
for(int i=5 ; i < sizeof(BITS) ; i++, P++, Z++)
{
    PACKET[i] = BITS[P];

    if( (A * B) == Z)   // A==1 B==24 Z==FOR_LOOP
    {
        I = 0;      // ZERO THIS OUT.  THIS IS USED FOR P COUNTER
        A++;        // Increase by one for IF_STATMENT formula
        if( (P+7) <= sizeof(BITS))  // if P+7 is less then or equal to BITS then its okay todo the following
        {                           // which is needed because maybe P+7 is bigger then BITS, so this wouldnt be good ...
            PACKET[i] = BITS[P+7];
            I++;
        }
        if( (P+6) <= sizeof(BITS))  // Explained Above
        {
            PACKET[i+1] = BITS[P+6];
            I++;
        }
   开发者_JAVA技巧     if( (P+5) <= sizeof(BITS))  // Explained Above
        {
            PACKET[i+2] = BITS[P+5];
            I++;
        }
        if( (P+4) <= sizeof(BITS))  // Explained Above
        {
            PACKET[i+3] = BITS[P+4];
            I++;
        }
        if( (P+3) <= sizeof(BITS))  // Explained Above
        {
            PACKET[i+4] = BITS[P+3];
            I++;
        }
        if( (P+2) <= sizeof(BITS))  // Explained Above
        {
            PACKET[i+5] = BITS[i+2];
            I++;
        }
        if( (P+1) <= sizeof(BITS))  // Explained Above
        {
            PACKET[i+6] = BITS[P+1];
            I++;
        }
        if( P <= sizeof(BITS))      // Explained Above
        {
            PACKET[i+7] = BITS[P];
            I++;
        }
        P += I;                 // We have been increasing I each time we do one of the statments to keep track of how many loops
        if(I != 0)              // we need to add to P to keep it on track ...  + I-1 to keep i on track
            i += I-1;
    }
}
}

    int main() 
    {

    const char p40[] =
    {
        0x81, 0x11, 0xb6, 0x1e, 0xc9, 0x67, 0x0e, 0x52, 0x0b, 0xec, 0xff, 0x3b, 0xa8, 0xfa, 0x2a, 0x62, 
        0x41, 0x79, 0xd2, 0x75, 0x7b, 0x93, 0xaf, 0xb4, 0xcf, 0x10, 0x3a, 0x12, 0x4d, 0x4b, 0x60, 0x64, 
        0xcc, 0x78, 0x01, 0xd1, 0x83, 0xbc, 0x27
    };

        Holder k;                               // Call our struct for holding stuff
        unsigned char bitbit[sizeof(p40)*8];    // bitbit is an array that is p40 * 8 to get the amount of bits being made using BIT function

        BIT(p40,sizeof(p40),(char*)bitbit);     // BIT takes p40, and turns it into bits and stores them in bitbit
        k.bits = bitbit;                        // now k.bits is equal to bitbit

    cout << k.bits;                             // dumps k.bits to screen
    cout << "\n";                               // space to see ...

        unsigned char r[sizeof(k.bits-11)];     // r is k.bits - 11 because 11 bits are used for something else

        READ(k.bits, k.header,k.hasid, k.cid,r);    //This puts k.bits into READ function.  Itll store the info into the struct

    cout << "\nPACKET_HEADER:\n";
    cout << k.header;                           // This dump seems to be displaying more then what its suppose too...

    cout << "\nHAS_CANNEL:\n";
    cout << k.hasid;                            // This dump seems to be displaying more then what its suppose too...

    cout << "\nCHANNEL_ID:\n";
    cout << k.cid;                              // This dump seems to be displaying more then what its suppose too...

    cout << "\nPACKET_BITS:\n";
    cout << r;                                  // The last dump (r) is displaying all inputs into the function READ + r.

      int a;                                    // Used to hang program ...
      cin >> a;
      return 0;
    }

---

You are holding your data in char arrays, and so when you pass them to std::cout the operator assumes null-terminated C strings. Unless your headers are guaranteed to be null-terminated, std::cout is going to run off the end of the arrays and print more than you want them to.

A kind of hacky way to fix this:

struct Holder
{
    unsigned char *bits;        // holds our bits
    unsigned char header[6];    // Always 6 bits
    unsigned char header_term = '\0' // null terminator
    unsigned char hasid[1];     // Always 1 bit
    unsigned char hasid_term = '\0' // null terminator
    unsigned char cid[4];       // always 4 bits
    unsigned char cid_term = '\0' // null terminator
};

Assuming your compiler doesn't re-order the fields in Holder, this should cause std::cout to print only the number of bytes you expect.

This answers only part of your problem. I'm posting for now and will edit if I figure out the rest.

ETA: Looking over READ, it appears your problem is again an absence of null-termination. You need one extra char in the array to hold the null character ('\0', near universally equal to NULL, itself almost always 0) which will tell std::cout that the C string is ending. Also, this is a cleaner way to provide null-termination to Holder:

struct Holder
{
    unsigned char *bits;        // holds our bits
    unsigned char header[7]={0,0,0,0,0,0,'\0'};    // Always 6 bits
    unsigned char hasid[2]={0,'\0'};     // Always 1 bit
    unsigned char cid[5]={0,0,0,0,'\0'};       // always 4 bits
};

I'm keeping the original, ugly fix above because it illustrates the rules that cause your code to print extra garbage.