0

Let's consider follwing code.

message_word.h

struct array_int64{
        int size;
        long* value;
};
struct array_int64* array_int64_create(int size);
int array_int64_get(void* value, const struct array_int64* array, int index);
int array_int64_set(struct array_int64* array, long value, int index);
void array_int64_free(struct array_int64* array);
struct hello_message{
        struct array_int64* test;
};
struct hello_message* hello_message_create();
void hello_message_free(struct hello_message* msg);

message_word.c

#include <stdlib.h>
#include <string.h>
#include "word_message.h"

struct array_int64* array_int64_create(int size){
        struct array_int64* new_array = (struct array_int64*)malloc(sizeof(struct array_int64));
        new_array->size = size;
        new_array->value = (long*)malloc(sizeof(long) * (new_array->size));
        return new_array;
}
int array_int64_get(void* value, const struct array_int64* array, int index){
        long* vvalue = (long*)value;
        if(index >= array->size)
                return -1;
        *vvalue = (array->value)[index];
        return 0;
}
int array_int64_set(struct array_int64* array, long value, int index){
        if(index >= array->size)
                return -1;
        array->value[index] = value;
        return 0;
}
void array_int64_free(struct array_int64* array){
        if(array != NULL && array->value != NULL)
                free(array->value);
        if(array != NULL)
                free(array);
}
struct hello_message* hello_message_create(){
        struct hello_message* new_msg = (struct hello_message*)malloc(sizeof(struct hello_message));
        new_msg->test = array_int64_create(5);
        return new_msg;
}
void hello_message_free(struct hello_message* msg){
        if(msg == NULL) return;
        array_int64_free(msg->test);
        free(msg);
}

main.c

#include "word_message.h"
#include <stdio.h>

//struct hello_message* msg = NULL;

int main(void)
{
        struct hello_message* msg = hello_message_create();
        //msg = hello_message_create();
        array_int64_set(msg->test, 10, 0);
        int number;
        array_int64_get(&number, msg->test, 0);
        printf("value is: %d\n", number);
        array_int64_get(&number, msg->test, 0);
        printf("value is: %d\n", number);
        hello_message_free(msg);
        return 0;
}

When I compile and run this code with gcc-9.3.0 or gcc-7.5.0 on Ubuntu the msg reference address changes itself when array_int64_get is executed. However, if you make msg a global variable like in the comment, it does not change.When I compile and run this code on Ubuntu 18, gcc-7.5.0 on a Raspberry Pi (ARM), everything works fine. It also works fine on windows. So is this a bug in gcc?

This is the result of running on ubuntu18(x86)+gcc-9.3.0 running on ubuntu18

This is the result of running it on a Raspberry Pi run on raspberry pi

Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
ljr_rjl
  • 3
  • 2
  • 2
    You are taking the address of an `int` and casting it to `long*`. That doesn't seem right. – Raymond Chen Jan 02 '22 at 03:34
  • 5
    `int number; array_int64_get(&number, msg->test, 0);` is incorrect. If you're going to treat `number` as a `long` inside the function it should also be one outside the function. You might consider using `int64_t` instead. Your code may work in Windows because `int` and `long` are the same size and neither is 64 bit. – Retired Ninja Jan 02 '22 at 03:35
  • 1
    As an aside, why would you want set to take a long, but then have the get use a void*, rather than a long*? It would reduce the likelyhood of this type of error. – Dave S Jan 02 '22 at 03:38
  • Sorry, I thought this problem would occur in either C or C++ so I labeled it C++, but of course this is a piece of C code – ljr_rjl Jan 02 '22 at 03:39
  • 1
    @RetiredNinja thank you for your answer sincerely.I think this is the root cause. – ljr_rjl Jan 02 '22 at 03:49
  • [Images](https://meta.stackoverflow.com/q/285551/90527) should not be used for textual data, such as output and error messages. – outis Jan 02 '22 at 15:53

1 Answers1

1
int array_int64_get(void* value, const struct array_int64* array, int index){
        long* vvalue = (long*)value;
        ...
 }

 int main() {
        int number;
        array_int64_get(&number, msg->test, 0);
 }

You are passing an int with a pointer and then reading it as long. That's invalid. If you want it long, be it long.

Use longs everywhere, and if your functions wants a long make it take a long.

int array_int64_get(long *vvalue, const struct array_int64* array, int index){
    ...
}

 int main() {
        int number;
        array_int64_get(&number, ...); // compiler warning!
        long correctnumber;
        array_int64_get(&correctnumber, ...); // all fine!
 }

Conceptually, your code is odd. long is not 64-bit long, it's at least 32-bits long. It can have 32-bits, 64-bits, 1000-bits. For 64-bit, use uint64_t or int64_t from #include <stdint.h>. See https://en.cppreference.com/w/c/types/integer .

free(NULL) is totally fine - no need to check for it.

Do I cast the result of malloc?

KamilCuk
  • 120,984
  • 8
  • 59
  • 111