0

I was wondering if you could help me overcome a hurdle I've run into with my C syntax. I have written the function:

binary_and_trim(char *password, unsigned *key1, unsigned *key2)

that has achieved the goal of converting a provided string into binary and trimmed off the leading zero. I have assigned my key1 and key2 pointers to the correct indexes. But then, when I return to the main function the values are all lost.

I believe that the problem is that when I pass the *key1/*key2 pointers to the function it only receives a copy of them. But, as I am new to C, I don't know how to fix it?

I created a for loop to help me test/debug.

#include <stdio.h>
#include <string.h>
void binary_and_trim(char *password, unsigned *key1, unsigned *key2);
unsigned int get_n_bits(unsigned *bits, int width, int index);


int main(int argc, const char * argv[]) {
    unsigned *key1 = NULL;
    unsigned *key2 = NULL;
    binary_and_trim("password", key1, key2);
    //This test fails with a EXC_BAD_ACCESS error
    for(int i = 0 ; i < 28; i++){
        printf("key1[%d] %u    key2[%d] %d\n", i, key1[i], i, (key2 + i));
    }
}

void binary_and_trim(char *password, unsigned *key1, unsigned *key2){
    char c;
    int count = 0;
    unsigned tmp;
    unsigned long len = strlen(password);
    unsigned trimmedbinary[len * 7];
    
    for(int i = 0; i < len; i++){
        c = *(password + i);
    for( int j = 6; j >= 0; j--) {
        tmp = 0;
        if(c >> j & 1){
            tmp = 1;
            }
        *(trimmedbinary + count) = tmp;
        count++;
    }
    }
    key1 = trimmedbinary;
    key2 = &trimmedbinary[28];
    
    //This test works correctly!!!
    for(int i = 0 ; i < 28; i++){
        printf("key1[%d] %d    key2[%d] %d\n", i, *(key1 + i), i, *(key2 + i));
    }
}
  • Does this answer your question? [Changing address contained by pointer using function](https://stackoverflow.com/questions/13431108/changing-address-contained-by-pointer-using-function) – kaylum Oct 07 '20 at 22:53

4 Answers4

2

I believe that the problem is that when I pass the *key1/*key2 pointers to the function it only receives a copy of them.

Yes, exactly. Pointers are just integers and integers get copied. You solve this with a pointer to a pointer, a "double pointer".

However, there is another problem. trimmedbinary is using stack/automatic memory. "Automatic" meaning it will be freed once the function exits. Once the function returns key1 and key2 will point at freed memory. trimmedbinary must be declared in heap/dynamic memory with malloc.

void binary_and_trim(char *password, unsigned int **key1, unsigned int **key2){
    unsigned int *trimmedbinary = malloc(len * 7 * sizeof(unsigned int));

    ...

    *key1 = trimmedbinary;
    *key2 = &trimmedbinary[28];

    for(int i = 0 ; i < 28; i++) {
        printf("key1[%d] %u, key2[%d] %u\n", i, (*key1)[i], i, (*key2)[i]);
    }

    return;
}

And call it as binary_and_trim("password", &key1, &key2);

Schwern
  • 153,029
  • 25
  • 195
  • 336
  • I took your advice and implemented the malloc for trimmedbinary. So, do I have to free that memory somewhere? Do I do it in main? Or no? Still struggling with working granularly with memory. – Learning-C Oct 08 '20 at 00:06
  • @Learning-C Yes, you would have to `free(key1)` once you're done with it. Do not free `key2` since it is pointing at that same memory. – Schwern Oct 08 '20 at 00:23
1

Update: I answered the question about how to alter the pointer value, but I have not noticed the memory issue in the code. Please refer to this answer instead.

Pointers are variables themselves. You may already know that with a pointer, you can change the value stored in the variable the pointer points to. Therefore, you need to use a pointer to a pointer to change the value (the memory address) stored in the pointer.

Change your function signature to:

void binary_and_trim(char *password, unsigned **key1, unsigned **key2)

Call with:

binary_and_trim("password", &key1, &key2);

and replace key1 and key2 to *key1 and *key2 in the function definition.

Yang Hanlin
  • 474
  • 1
  • 6
  • 18
1

Your problem is that the variable you use to fill with your keys data trimmedbinary is allocated only for the scope of the function binary_and_trim. That said, when you print inside the function

void binary_and_trim(char *password, unsigned **key1, unsigned **key2){
    ...
    unsigned trimmedbinary[len * 7]; // <--
    
    ...

    *key1 = trimmedbinary; // <--
    *key2 = &trimmedbinary[28]; // <--
    
    //This test works correctly!!!
    for(int i = 0 ; i < 28; i++){
        printf("key1[%d] %d    key2[%d] %d\n", i, *(key1 + i), i, *(key2 + i));
    }
}

it just works because the data your key1 pointer is trying to access is still there. However, when you return from your function back to main, key1 and key2 still point back to the buffer you initialized inside binary_and_trim, which is no longer valid because is out of scope.

I suggest you create a buffer in main and pass it as a parameter,

int main(int argc, const char * argv[]) {
    const char* password = "password";
    unsigned long len = strlen(password);
    unsigned buffer[len * 7]; // <-- Add buffer here
    unsigned *key1 = NULL;
    unsigned *key2 = NULL;
    binary_and_trim(password, &key1, &key2, &buffer, len * 7);

    //This test succeeds
    for(int i = 0 ; i < 28; i++){
        printf("key1[%d] %u    key2[%d] %d\n", i, key1[i], i, (key2 + i));
    }
}

void binary_and_trim(char *password, unsigned **key1, unsigned **key2, unsigned** buffer, size_t buff_size){
    char c;
    int count = 0;
    unsigned tmp;
    ...
    //Use *buffer instead of trimmedbinary

    //Check if buff_size matches len(password) * 7

or alternatively, make the buffer heap allocated (dont forget to free() later).

I believe that the problem is that when I pass the *key1/*key2 pointers to the function it only receives a copy of them.

Already altered in code as well.

Primemaster
  • 312
  • 5
  • 16
  • @AndreasWenzel Yes you are right it should be passed. Although in this case one could assume to always pass a buffer with the size of the password, still being unsafe no doubt. – Primemaster Oct 07 '20 at 23:24
  • I had deleted my comment before you posted your comment, because I noticed that you do ensure that the buffer is sufficient in size. Generally, I think it is a good idea that functions that receive a buffer as a function argument should also receive a function argument with the size of the buffer, so that the function can ensure that no [buffer overflow](https://en.wikipedia.org/wiki/Buffer_overflow) occurs. However, in this case it is maybe not necessary. – Andreas Wenzel Oct 07 '20 at 23:29
  • @AndreasWenzel I edited the code as it seems totally reasonable to check the buffer size, else you can detect if any error occurs by using a wrong buffer, for ex. always passing `sizeof(buffer)`. – Primemaster Oct 07 '20 at 23:31
0

Wow! Thank you EVERYONE! I finally got it up and running (after 4 hours of beating my head against the wall). I can't begin to say how clutch you all are. I'm realizing I have tons to learn about the granular memory access of C (I'm used to Java). I can't wait to be an actual WIZARD like you all!

  • Yes, in C, you must be much more careful with handling memory than in other languages. In most other languages, the program will do the memory management for you and warn you immediately when you do something wrong. However, in C, even if you do something wrong, it will sometimes work and sometimes not, so debugging is much harder. On the other hand, the advantage of C is that the code can run faster (if you program it efficiently), because your program doesn't have to do the memory management for you. – Andreas Wenzel Oct 07 '20 at 23:35