1

Why does this double mapping array almost work, but doesn't?

My code is as follows:

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

typedef struct {
    double mapping [3][3];
} CoordinateMapperStr;
typedef CoordinateMapperStr * CoordinateMapper;

CoordinateMapper CoordinateMapper_Constructor(void)
{
    CoordinateMapper this = (CoordinateMapper) calloc (1, sizeof(CoordinateMapper));
    //return this; // <- I was missing this return, but still the rest worked the same
}

void CoordinateMapper_Initialize(CoordinateMapper this, double numb)
{
    for (int i=0; i < 3; i=i+1) {
        for (int j=0; j < 3; j=j+1) {
            this->mapping[i][j] = numb;
            printf("mapping(%d, %d) = %f\n", i, j, this->mapping[i][j]);
        }
    }
}

void CoordinateMapper_Print(CoordinateMapper this)
{
    for (int i=0; i < 3; i=i+1) {
        for (int j=0; j < 3; j=j+1) {
            printf("mapping(%d, %d) = %f\n", i, j, this->mapping[i][j]);
        }
    }
}

int main()
{
    CoordinateMapper mapper_1 = CoordinateMapper_Constructor();
    CoordinateMapper_Initialize(mapper_1, 1);
    printf("Init 1 done\n");

    CoordinateMapper_Print(mapper_1);
    printf("Print 1 done\n");

    CoordinateMapper mapper_2 = CoordinateMapper_Constructor();
    CoordinateMapper_Initialize(mapper_2, 2);
    printf("Init 2 done\n");

    CoordinateMapper_Print(mapper_1);
    printf("Second print 1 done\n");

    CoordinateMapper_Print(mapper_2);
    printf("Print 2 done\n");
}

// Here is the corresponding output
user:~/path$ gcc src/test_3.c -o test_3
user:~/path$ ./test_3
mapping(0, 0) = 1.000000
mapping(0, 1) = 1.000000
mapping(0, 2) = 1.000000
mapping(1, 0) = 1.000000
mapping(1, 1) = 1.000000
mapping(1, 2) = 1.000000
mapping(2, 0) = 1.000000
mapping(2, 1) = 1.000000
mapping(2, 2) = 1.000000
Init 1 done
mapping(0, 0) = 1.000000
mapping(0, 1) = 1.000000
mapping(0, 2) = 1.000000
mapping(1, 0) = 1.000000
mapping(1, 1) = 0.000000 // This is not correct
mapping(1, 2) = 0.000000 // This is not correct
mapping(2, 0) = 0.000000 // This is not correct
mapping(2, 1) = 1.000000
mapping(2, 2) = 1.000000
Print 1 done
mapping(0, 0) = 2.000000
mapping(0, 1) = 2.000000
mapping(0, 2) = 2.000000
mapping(1, 0) = 2.000000
mapping(1, 1) = 2.000000
mapping(1, 2) = 2.000000
mapping(2, 0) = 2.000000
mapping(2, 1) = 2.000000
mapping(2, 2) = 2.000000
Init 2 done
mapping(0, 0) = 1.000000
mapping(0, 1) = 1.000000
mapping(0, 2) = 1.000000
mapping(1, 0) = 1.000000
mapping(1, 1) = 0.000000 // This is not correct
mapping(1, 2) = 0.000000 // This is not correct
mapping(2, 0) = 0.000000 // This is not correct
mapping(2, 1) = 1.000000
mapping(2, 2) = 1.000000
Second print 1 done
mapping(0, 0) = 2.000000
mapping(0, 1) = 2.000000
mapping(0, 2) = 2.000000
mapping(1, 0) = 2.000000
mapping(1, 1) = 2.000000
mapping(1, 2) = 2.000000
mapping(2, 0) = 2.000000
mapping(2, 1) = 2.000000
mapping(2, 2) = 2.000000
Print 2 done
  1. What is the proper way to setup a double array within a struct pointer?
  2. Why does each struct pointer seem to make it´s own new array, but still they are a bit flaky?
  3. What gcc compiler flags could I use to help me see this kind of error and the missing return this; in the constructor?
Thor Tomasarson
  • 64
  • 1
  • 7
  • I could use 'double* mapping [3];' and then iterate over 'this->mapping[i] = (double*)malloc(c * sizeof(double));' in the constructor. But is that necessary, and the easiest solution? – Thor Tomasarson Nov 29 '21 at 23:13
  • 2
    `typedef CoordinateMapperStr * CoordinateMapper;` it is a very, very, very ..... bad practice to hide pointer s behind typedefs. **Never** do it. – 0___________ Nov 29 '21 at 23:21
  • 3
    `CoordinateMapper this = (CoordinateMapper) calloc (1, sizeof(CoordinateMapper));` --> `CoordinateMapper this = (CoordinateMapper) calloc (1, sizeof(*this));`. `CoordinateMapper` is a pointer, you're not allocating the appropriate amount of space (in this case much less) for your struct. – yano Nov 29 '21 at 23:23
  • Why do you say that it is "very, very, very.... bad"? Could you point me to a book or reading material for object oriented programming practices for C? For example here [link](http://staff.washington.edu/gmobus/Academics/TCES202/Moodle/OO-ProgrammingInC.html) they give a compelling argument for typedef on a pointer. – Thor Tomasarson Nov 29 '21 at 23:25
  • @ThorTomasarson It's bad practice because it makes mistakes like yours more likely. – Yakov Galka Nov 29 '21 at 23:29

2 Answers2

1

The problem is that whether you return this or not, you get undefined behavior in either case.

When you don't return this your non-void function doesn't return a value -- thus your code uses some garbage value (which might happen to be the return value from calloc).

If you return this -- you return allocation of sizeof(CoordinateMapper), which is just a size of a single pointer. This is less than your struct sizeof(CoordinateMapperStr), and your other code reads/writes beyond the allocated memory. This is, again, undefined behavior.

Yakov Galka
  • 70,775
  • 16
  • 139
  • 220
  • Is there any gcc compiler argument or anything similar that could give me warnings when I do these kinds of obvious errors? – Thor Tomasarson Nov 29 '21 at 23:29
  • 1
    @ThorTomasarson `-Wall` will warn you about the missing `return`. I don't think there's any GCC flag that will warn about the wrong `sizeof`. You can use `valgrind` or some static analysis tools to detect that, however. – Yakov Galka Nov 29 '21 at 23:33
  • @ThorTomasarson if you use the coding style paradigm from my comment then you'll at least always have the right base object size (how many of those objects to allocate space for must still be correct). I wrote [an answer](https://stackoverflow.com/a/70073561/3476780) about this last week with some more details if you're interested, and I'm sure further examples/discussion can be found on SO. Actually the answer with the pictures there is probably the best one. – yano Nov 29 '21 at 23:37
1

@YakovGalka spotted my error. I want to add here that valgrind is indeed a tool that can detect these kinds of programming errors. By adding -Wall and -g to gcc as compiler flags and running the application with valgrind ./compiled_app then these kinds of errors are easily detected.

Thor Tomasarson
  • 64
  • 1
  • 7