2

I am trying to convert an arbitrary buffer to a string of its binary representation. I was looking at some code from here: http://snippets.dzone.com/posts/show/2076 in order to get started. I realize that this code can not convert an arbitrary buffer, but only the specific case of an int; however, I figured that I could adapt it to any case once it was working.

The problem is that it returns some strange symbols (like this: �왿") instead of the binary. Does anyone either know what is wrong with this code specifically or explain how to convert an arbitrary buffer?

Please keep in mind that I am new to c++.

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

char *getBufferAsBinaryString(void *in)
{
    int pos=0;
    char result;
    char bitstring[256];
    memset(bitstring, 0, 256);
    unsigned int *input= (unsigned int *)in;
    for(int i=31;i>=0;i--)
    {
            if (((*input >> i) & 1)) result = '1';
            else result = '0';
            bitstring[pos] = result;
            if ((i>0) && ((i)%4)==0)
            {
                    pos++;
                    bitstring[pos] = ' ';
            }
            pos++;
    }
    return bitstring;
}
int main(int argc, char* argv[])
{
    int i=53003;
    char buffer[1024];
    char *s=getBufferAsBinaryString(&i);
    strcpy(buffer, s);
    printf("%s\n", buffer);
}
  • 3
    The behavior of this program is undefined because you're returning and then using a pointer to a stack-allocated buffer. You should be glad the computer didn't magically turn you into a warthog. It's also C, rather than C++. – Fred Foo Jun 22 '11 at 21:20
  • Among other things `getBufferAsBinaryString` has undefined behavior since it returns a pointer to a local. – Mark B Jun 22 '11 at 21:20
  • If you have a buffer `char buf[N];` you can make a string via `std::string s(buf, buf + N);`. – Kerrek SB Jun 22 '11 at 21:40
  • two `main`s? was that a copy / paste hiccup? :) – pmg Jun 22 '11 at 21:58
  • Why are you passing `i` as a `void *`? – Karl Knechtel Jun 22 '11 at 22:14
  • What do you expect as the output? – Omnifarious Jun 22 '11 at 22:53
  • @pmg: You couldn't fix it? I know you have the rep. –  Jun 23 '11 at 12:14
  • @0A0D: I could; I just didn't want to have the trouble of checking the program (maybe the OP intended to have a `main2` function and typed its name wrong, pay attention to the order of the functions, understand why the OP needed to include a non-standard header, why the code is limited to C99, ..., ...) – pmg Jun 23 '11 at 12:27

3 Answers3

2

The array bitstring has what is known as automatic duration, which means that it springs into existence when the function is called and disappears when the function returns.

Therefore, the pointer that this version of getBufferAsBinaryString returns is to an array which no longer exists by the time the caller receives the pointer. (Remember that the statement return bitstring; returns a pointer to the first character in bitstring; by the "equivalence of arrays and pointers," the mention of the array bitstring in this context is equivalent to &bitstring[0].)

When the caller tries to use the pointer, the string created by getBufferAsBinaryString might still be there, or the memory might have been re-used by some other function. Therefore, this version of getBufferAsBinaryString is not adequate and not acceptable. Functions must never return pointers to local, automatic-duration arrays.

Since the problem with returning a pointer to a local array is that the array has automatic duration by default, the simplest fix to the above non-functional version of getBufferAsBinaryString is to declare the array static, instead:

char *getBufferAsBinaryString(void *in)
{
    int pos=0;
    char result;
    static char bitstring[256];
    memset(bitstring, 0, 256);
    unsigned int *input= (unsigned int *)in;
    for(int i=31;i>=0;i--)
    {
            if (((*input >> i) & 1)) result = '1';
            else result = '0';
            bitstring[pos] = result;
            if ((i>0) && ((i)%4)==0)
            {
                    pos++;
                    bitstring[pos] = ' ';
            }
            pos++;
    }
    return bitstring;
}

Now, the bitstring array does not disappear when getBufferAsBinaryString returns, so the pointer is still valid by the time the caller uses it.

Returning a pointer to a static array is a practical and popular solution to the problem of "returning" an array, but it has one drawback. Each time you call the function, it re-uses the same array and returns the same pointer. Therefore, when you call the function a second time, whatever information it "returned" to you last time will be overwritten. (More precisely, the information, that the function returned a pointer to, will be overwritten.)

Although the static return array technique will work, the caller has to be a little bit careful, and must never expect the return pointer from one call to the function to be usable after a later call to the function

But you still have a different problem with passing in void * which I did not cover, in addition not passing in the size of your buffer. Since you shouldn't assume that your array ends in the \0, you should also pass in the size of your buffer.

1

Ignoring the other issues with memory and safety, you are not returning a valid null terminated string. Nor do you pass in the size of the input buffer, so you really just print out the 32-bit bit representation of the first 32-bits of the input buffer.

So, in addition to passing in a char buffer to write to or simply returning a std::string, you should also pass in the size of the input buffer and loop over that as well.

MSN
  • 53,214
  • 7
  • 75
  • 105
0

You can't return locally-allocated arrays from a function. As soon as the function finishes, the array ceases to exist, so the pointer you've returned no longer points to a valid object.

Instead of using a char array to represent your bitstring, you should consider using std::string, which has sensible value semantics (i.e., you can copy it, return it from a function, etc, without having to worry about it except from an efficiency standpoint).

Josh
  • 992
  • 5
  • 5
  • You can if it is static :) By default, locally allocated arrays have automatic duration. –  Jun 23 '11 at 12:10