memcpy(), uninitialized local variable

Question

Initially, i ran this code on ubuntu and it worked just fine without any warnings whatsoever. However, when I run it on VS on windows it says _operand1 is not initialized. I'm wondering how it can go wrong.

I know about not casting results of malloc, but VS just keeps throwing warnings.

Program is supposed to take char array of 9 bytes. First byte represents arithmetic operation, and other 8 represent 2 ints 4 bytes each (4-digit numbers). Here is the code:

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

float* calculate(char *, int *, int *);

int main() {

    char buffer[9];

    gets_s(buffer);

    int a, b;

    float* rez = calculate(buffer, &a, &b);

    printf("Operand1: %d, Operand 2: %d\n Result: %f\n", a, b, *rez);

    return 0;
}

float* calculate(char *buffer, int *a, int *b) {
    char operation;
    char *_operand1;
    char *_operand2;

    int operand1, operand2;

    memcpy(_operand1, buffer + 1, sizeof(int));
    _operand2 = (buffer + 5);

    operand1 = atoi(_operand1);
    operand2 = atoi(_operand2);

    operation = buffer[0];

    float *rez = (float *)malloc(sizeof(float));

    switch (operation) {
    case '0':
        *rez = (float)(operand1 + operand2);
        break;
    case '1':
        *rez = (float)(operand1 - operand2);
        break;
    case '2':
        *rez = (float)(operand1 * operand2);
        break;
    case '3':
        *rez = (float)operand1 / operand2;
        break;
    }

    return rez;

}

Answer 1

I really don't know what you expect to happen, but this is 100% wrong.

char *_operand1; /* uninitialized */
char *_operand2; /* uninitialized */

int operand1, operand2;

/* _operand1 is still uninitialized... */
memcpy(_operand1, buffer + 1, sizeof(int));

Nothing good can happen when you call memcpy() here. The absolute best-case scenario is that your program will crash. However, it might not crash, and that is an awful thing to ponder… if it's not crashing, what is it doing? It's probably doing something you don't want it to do.

Further analysis

The code is very suspicious in general.

memcpy(_operand1, buffer + 1, sizeof(int));

Why sizeof(int)? buffer is a pointer to an array of characters, presumably, and there's no particular reason to choose sizeof(int), since the result is just passed into atoi. The atoi function just takes a pointer to an NUL-terminated array of characters, sizeof(int) is just the size of the result of atoi not the size of the input.

How to make it work

Here is how you would do this without invoking undefined behavior:

char tmp[5];

memcpy(tmp, buffer, 4); // don't use sizeof(int), just use 4
tmp[4] = '\0'; // add NUL terminator
int op0 = atoi(tmp);

memcpy(tmp, buffer + 5, 4);
// NUL terminator already present in tmp
int op1 = atoi(tmp);

Note that when you pass tmp to memcpy, it is converted to a pointer of type char * that points to the first element of tmp. Kind of like doing the following:

char tmp[5];
char *my_ptr = &tmp[0];
memcpy(my_ptr, ...);

You can see how this is different from:

char *my_ptr; // uninitialized
memcpy(my_ptr, ...);

Answer 2

when I run it on VS on windows it says _operand1 is not initialized. I'm wondering how it can go wrong.

Using an uninitialized value results in undefined behavior, which means anything is allowed to happen. That's how it can go wrong.

---

I know about not casting results of malloc, but VS just keeps throwing warnings.

Are you by any chance compiling your code as C++? Because C++ doesn't allow implicit conversions from void*, whereas C does.