Search code examples
cpointersbinarysizememcpy

C - Memncpy/Strncpy (tried strncat too) copies 1 character less than it should, help is appreciated


Like I said, memncpy() (in the middle of main) copies 1 character less than it should, not sure why. I added comments and an image to make it more understandable.

    #define BIT_AMOUNT 4

char * randomBinaryGenerator(char * random){

    int randomNum, i;
    char temp;
    char * tempPtr = (char *)malloc(1 * sizeof(char));

    for(i = 0; i <= BIT_AMOUNT - 1; i++){

        randomNum = rand() % 2;
        temp = randomNum + '0';

        tempPtr = NULL;
        tempPtr = &temp;
        strcat(random, tempPtr);
    }

    return random;
}

int main(){

    srand(time(0));
    char str_bin_bitKey[BIT_AMOUNT] = "";

    char * random = (char *)malloc(BIT_AMOUNT * sizeof(char));
    char * bin_bitKey = (char *)malloc(BIT_AMOUNT * sizeof(char));

    printf("\nSize of str_bin_bitKey: %ld, Size of bin_bitKey: %ld\n", sizeof(str_bin_bitKey), sizeof(bin_bitKey));

    bin_bitKey = randomBinaryGenerator(random);    //generates 4 bit long binary number
    memcpy(str_bin_bitKey, bin_bitKey, BIT_AMOUNT);//copies 1 character less

    printf("\nbin_bitKey:     %s\n", bin_bitKey);    //4 bits
    printf("\nstr_bin_bitKey: %s\n", str_bin_bitKey);//3 bits???

    long long dec_bitKey = 0;//unimportant for now .... convertBinaryToDecimal(bin_bitKey); 

    printf("\ndec_bitKey: %lld\n\n", dec_bitKey);

    free(random);
    return 0;
}

This is what the output looks like, As you can see str_bin_bitKey is 3 characters instead of 4: As you can see str_bin_bitKey is 3 characters instead of 4

I appreciate all the help.


Solution

  • A few issues ...

    In main, the size of the arrays/pointers need to allow for the nul terminator, so they need to be BIT_AMOUNT + 1

    In main, your memcpy does not copy the nul terminator. Use strcpy instead.

    Adding a starting nul is most easily done with (e.g.):

    *random = 0;
    

    Don't cast malloc: Do I cast the result of malloc?

    sizeof(char) is always 1 (by definition), regardless of the actual, architecture dependent size (e.g. char is actually 16 bits). So, don't use sizeof(char)

    In randomBinaryGenerator, tempPtr leaks memory. No need for malloc [or even tempPtr at all]. Use char temp[2]; instead.

    sizeof(bit_bitKey) is always constant, because it's the size of the pointer and not what it points to (i.e. it is not BIT_AMOUNT).

    randomBinaryGenerator needs almost a complete rework.


    Here's an annotated and fixed version of your code. I've added:

    #if 0
    // old/original code
    #else
    // new/fixed code
    #endif
    

    to help show the changes.

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <time.h>
    
    #define BIT_AMOUNT 4
    
    char *
    randomBinaryGenerator(char *random)
    {
    
        int randomNum, i;
    #if 0
        char temp;
        char *tempPtr = malloc(1);
    #else
        char temp[2];
    #endif
    
    #if 1
        // add nul terminator
        *random = 0;
        temp[1] = 0;
    #endif
    
    #if 0
        for (i = 0; i <= BIT_AMOUNT - 1; i++) {
    #else
        for (i = 0; i < BIT_AMOUNT; i++) {
    #endif
            randomNum = rand() % 2;
    #if 0
            temp = randomNum + '0';
            tempPtr = NULL;
            tempPtr = &temp;
            strcat(random, tempPtr);
    #else
            temp[0] = randomNum + '0';
            strcat(random, temp);
    #endif
        }
    
        return random;
    }
    
    int
    main(void)
    {
    
        srand(time(0));
    
    // NOTE/BUG: need space for EOS terminator
    #if 0
        char str_bin_bitKey[BIT_AMOUNT] = "";
        char *random = malloc(BIT_AMOUNT);
        char *bin_bitKey = malloc(BIT_AMOUNT);
    #else
        char str_bin_bitKey[BIT_AMOUNT + 1] = "";
        char *random = malloc(BIT_AMOUNT + 1);
        char *bin_bitKey = malloc(BIT_AMOUNT + 1);
    #endif
    
    // NOTE/BUG: sizeof(bit_bitKey) is the size of the _pointer_ and _not_ what
    // it points to (i.e. it is _not_ BIT_AMOUNT)
    #if 0
        printf("\nSize of str_bin_bitKey: %ld, Size of bin_bitKey: %ld\n",
            sizeof(str_bin_bitKey), sizeof(bin_bitKey));
    #endif
    
        // generates 4 bit long binary number
        bin_bitKey = randomBinaryGenerator(random);
    
        // copies 1 character less
    #if 0
        memcpy(str_bin_bitKey, bin_bitKey, BIT_AMOUNT);
    #else
        strcpy(str_bin_bitKey, bin_bitKey);
    #endif
    
        // 4 bits
        printf("\nbin_bitKey:     %s\n", bin_bitKey);
        // 3 bits???
        printf("\nstr_bin_bitKey: %s\n", str_bin_bitKey);
    
        // unimportant for now .... convertBinaryToDecimal(bin_bitKey);
        long long dec_bitKey = 0;
    
        printf("\ndec_bitKey: %lld\n\n", dec_bitKey);
    
        free(random);
    
        return 0;
    }
    

    Here's a cleaned up and improved version. Note that randomBinaryGenerator is faster/better without using strcat at all.

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <time.h>
    
    #define BIT_AMOUNT 4
    
    char *
    randomBinaryGenerator(char *random)
    {
    
        int randomNum, i;
    
        for (i = 0; i < BIT_AMOUNT; i++) {
            randomNum = rand() % 2;
            random[i] = randomNum + '0';
        }
    
        // add nul terminator
        random[i] = 0;
    
        return random;
    }
    
    int
    main(void)
    {
    
        srand(time(0));
    
        char str_bin_bitKey[BIT_AMOUNT + 1];
        char *random = malloc(BIT_AMOUNT + 1);
        char *bin_bitKey = malloc(BIT_AMOUNT + 1);
    
        // generates 4 bit long binary number
        bin_bitKey = randomBinaryGenerator(random);
    
        strcpy(str_bin_bitKey, bin_bitKey);
    
        // 4 bits
        printf("\nbin_bitKey:     '%s'\n", bin_bitKey);
    
        // 3 bits???
        printf("\nstr_bin_bitKey: '%s'\n", str_bin_bitKey);
    
        // unimportant for now .... convertBinaryToDecimal(bin_bitKey);
        long long dec_bitKey = 0;
    
        printf("\ndec_bitKey: %lld\n\n", dec_bitKey);
    
        free(random);
    
        return 0;
    }