1

I have the following code to convert a char array (like 'hi\n') to binary and store in an int array.

void data_to_binary(char *data){

bit_pos = 0;
int i = 0, j = 0, k = 0;
char mask = 0x80;

for(i = 0; i < sizeof(data); ++i){
    if(data[i] != '\0' && data[i] != '\n'){
        mask = 0x80;
        for(j = 0; j < 8; ++j){
            binary_data[bit_pos] = ((data[i] & mask) > 0);
            mask >>= 1;
            bit_pos++;
        }
    } 
}
}

This worked perfectly. I was getting 01101000 01101001 for hi. I changed NOTHING about this code and ran it again just recently and I am now getting 01111111 01111111.... I have no idea what is going on. While fiddling with unrelated code I did get a heap corruption error. Is that what is causing this? That it is still negatively impacting my code?

Tyler Dahle
  • 77
  • 1
  • 8
  • 4
    `sizeof(data)` is wrong. That does not give you the size of the array, only the size of the pointer. If `data` is a C string (ie, NUL terminated) then use `strlen(data)`. Otherwise you will need to pass in the array length into the function. – kaylum Sep 30 '16 at 05:53
  • I unfortunately will not know the size of array. It is user inputted with a maximum of 1024 characters. And it equals 2, so for the case of 'hi' it actually works. So that won't solve my issue. – Tyler Dahle Sep 30 '16 at 05:55
  • `strlen?` maybe? or `n = 0; while (*data) n++;`? – David C. Rankin Sep 30 '16 at 05:55
  • 1
    The code isn't "working code" just because it happens to produce the right result sometimes. What you have is wrong. Fix it. If it works by accident then that's pure luck and you should not rest your laurels on that. – kaylum Sep 30 '16 at 05:55
  • buuuuuut that wouldn't be the cause of it randomly spitting out the wrong binary for no reason. – Tyler Dahle Sep 30 '16 at 05:57
  • 2
    Yes it may! It's called Undefined Behaviour - [Does “Undefined Behavior” really permit *anything* to happen?](http://stackoverflow.com/questions/32132574/does-undefined-behavior-really-permit-anything-to-happen). Why argue? Your code is wrong. Fix it and then work from there. It is pointless discussing code that has an obvious bug in it. – kaylum Sep 30 '16 at 05:58
  • Yep, changed to strlen and still get the same issue. That wasn't the problem. Because sizeof(data) was ALWAYS 2...so it worked perfectly fine with 'hi' and was not causing undefined behavior. Strlen will be correct to make it work beyond strings of size 2. But that wasn't the issue. – Tyler Dahle Sep 30 '16 at 06:03
  • Unless you have a non-standard system `sizeof(data)` will be 4 or 8. Because that is the size of a pointer on most systems. Which would cause it to run past the valid `hi` data and hence the UB. And is your data really a string that allows you to use `strlen`? – kaylum Sep 30 '16 at 06:06
  • Yes. I ended up finding the issue..... I did overlook a change I made. mask was originally an int. It works when mask is an int. Well that was a long, frustrating time. – Tyler Dahle Sep 30 '16 at 06:08
  • 1
    Ah yes, right shifting a signed number duplicates the sign bit, and 0x80 *is* the sign bit for a `char`. – user3386109 Sep 30 '16 at 06:09
  • What are you talking about undefined behaviour? This is a snipplet of code with errors, some parts are missing, but it doesn't involve any undefined behaviour... – V-X Sep 30 '16 at 06:11
  • 2
    @user3386109 right-shifting a negative number causes implementation-defined behaviour. Also, `char` cannot be shifted per se, it undergoes promotion to `int` before shifting – M.M Sep 30 '16 at 06:17
  • @V-X this is not a complete program, there are several ways undefined behaviour could be involved . E.g. `data` or `binary_data` could overflow. Or there may be UB in code not shown here which happens to affect execution of this snippet – M.M Sep 30 '16 at 06:18
  • 2
    @user3386109 lack of understanding this "pedantry" is what led to OP's problems – M.M Sep 30 '16 at 06:23
  • No, lack of understanding how right shifting works with 2's complement numbers is what led to OP's problem. And your comment does nothing to improve the OP's knowledge of 2's complement numbers. – user3386109 Sep 30 '16 at 06:26

1 Answers1

2
  1. What do you expect the sizeof(data) to be? Wouldn't you use the strlen instead?
  2. You are using signed char for the mask. After first execution of "mask >>= 1;" you get 0xC0 instead of 0x40. Try using int or unsigned char, in order to avoid the sign to be set after the shift.
V-X
  • 2,979
  • 18
  • 28