1

What is wrong in my code below?

There are no errors or warnings when I compile it by Dev C++ compiler. But after I run my program there is execution error and following return value text:

Process exited after 5.1 seconds with return value 3221225477
Press any key to continue . . .

Any idea what is wrong?

When I use debug feature, error occurs in that line:

printf("Value of (*pointerToMyOwnStructPointer)->a = %d\n", (*pointerToMyOwnStructPointer)->a);

My code is:

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

typedef struct{
    int a;
    int b;
}myIntegers_t;

int main (void)
{
    myIntegers_t *myOwnStructPointer = NULL;
    myIntegers_t **pointerToMyOwnStructPointer = NULL;

    myOwnStructPointer = (myIntegers_t*)malloc(sizeof(myIntegers_t));

    if (myOwnStructPointer > 0)
    {   
        myOwnStructPointer->a = 2;
        myOwnStructPointer->b = 8;

        printf("Value of myOwnStructPointer->a = %d\n", myOwnStructPointer->a);
        printf("Value of myOwnStructPointer->b = %d\n", myOwnStructPointer->b);

        pointerToMyOwnStructPointer = (myIntegers_t**)myOwnStructPointer;

        printf("\n");
        printf("Value of (*pointerToMyOwnStructPointer)->a = %d\n", (*pointerToMyOwnStructPointer)->a);
        printf("Value of (*pointerToMyOwnStructPointer)->b = %d\n", (*pointerToMyOwnStructPointer)->b);  
    }
    else
    {
        return -1;
    }

    return 0;
}
Yu Hao
  • 119,891
  • 44
  • 235
  • 294
Mika
  • 11
  • 1
  • 3
  • 1
    Please [see why not to cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. – Sourav Ghosh Jul 15 '15 at 11:07
  • You can only compare pointers to related pointers (i.e. two pointersthat are both pointing to the same array) or to `NULL`. And if you compare with `NULL` you can only check for equality or inequality, not any other relational operator. – Some programmer dude Jul 15 '15 at 11:10
  • `if (myOwnStructPointer > 0)` causes undefined behaviour. It should be `if (myOwnStructPointer != 0)` – M.M Jul 16 '15 at 00:31

5 Answers5

3

in your code,

 pointerToMyOwnStructPointer = (myIntegers_t**)myOwnStructPointer;

is very wrong. You need to change that to

pointerToMyOwnStructPointer = &myOwnStructPointer;

to get the expected behaviour.

To elaborate,

  • myOwnStructPointer is a pointer-to-type.
  • pointerToMyOwnStructPointer is a pointer-to-pointer-to type.

they are not equivalent. Just because they are both pointers, you cannot simply cast value of one type to another and expect that to work. The cast is wrong (and not even required). So,

  • Do not cast
  • Enable compiler warnings.

Most of the time, your compiler will save you, through a warning, at least.


Note: Please see why not to cast the return value of malloc() and family in C.

Community
  • 1
  • 1
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
1

There would be errors or warnings if you didn't have that cast in the assignment

pointerToMyOwnStructPointer = (myIntegers_t**)myOwnStructPointer;

That's because myOwnStructPointer is a pointer to your structure, but pointerToMyOwnStructPointer is a pointer to a pointer to your structure, and those two types are totally incompatible.

The compiler will interpret it like this:

+-----------------------------+    +--------------+    +--------+
| pointerToMyOwnStructPointer | -> | some address | -> | struct |
+-----------------------------+    +--------------+    +--------+

So when you do your assignment and then use pointerToMyOwnStructPointer it lead to undefined behavior since it's not pointing to a pointer.


The lesson here is that you should almost never insert type-casting to solve compiler errors or warnings, the errors and warnings are there for a reason, and warnings often tell you that you are doing something suspect that might lead to undefined behavior.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
0

Only an improvement suggestion

Also, the correct malloc way of allocation would be

myIntegers_t * myOwnStructPointer = malloc(sizeof(myIntegers_t));
if (myOwnStructPointer == NULL)
    return 1;

. . .    // else do something with myOwnStructPointer;
Shreevardhan
  • 12,233
  • 3
  • 36
  • 50
0

This was my original code and now it is working after I added one '&' mark. Thanks a lot of your support!!

Any comments about this code? I think there is cast for the return value of malloc function... is that problem?

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

#define OK 1
#define FAILURE 0

typedef struct{
    int a;
    int b;
}myIntegers_t;

void * open(void);
int close(void ** pointerToMyOwnStructPointer );

void main(void)
{
    void * myVoidParameter;

    if(myVoidParameter = open())
    {
        printf("\n\nPORT OPENED SUCCESSFULLY!!\n\n");

        myIntegers_t ** pointerToMyVoidParameter = (myIntegers_t**)&myVoidParameter;

        printf("\n");
        printf("Value of pointerToMyVoidParameter->a = %d\n", (*pointerToMyVoidParameter)->a);
        printf("Value of pointerToMyVoidParameter->b = %d\n", (*pointerToMyVoidParameter)->b);

        if(close(&myVoidParameter))
        {
            printf("\n\nPORT CLOSED SUCCESSFULLY!!\n");
        }
        else
        {
            printf("\n\nFAILURE IN PORT CLOSING!!!\n");
        }
    }
    else
    {
        printf("\n\nFAILURE IN PORT OPENING!!!\n");
    }
}

void * open(void){

    myIntegers_t *myOwnStructPointer = (myIntegers_t*)malloc(sizeof(myIntegers_t));

    if (myOwnStructPointer)
    {   
        myOwnStructPointer->a = 2;
        myOwnStructPointer->b = 8;

        printf("\n");
        printf("Value of myOwnStructPointer->a = %d\n", myOwnStructPointer->a);
        printf("Value of myOwnStructPointer->b = %d\n", myOwnStructPointer->b);

        return myOwnStructPointer;
    }
    else
    {
        return NULL;
    }
}

int close(void ** pointerToMyOwnStructPointer ){
    int result = OK;
    myIntegers_t ** descriptor = (myIntegers_t**)pointerToMyOwnStructPointer;

    if(descriptor == NULL || *descriptor == NULL)
    {
        return FAILURE;
    }

    printf("\n");
    printf("Value of descriptor->a = %d\n", (*descriptor)->a);
    printf("Value of descriptor->b = %d\n", (*descriptor)->b);


    (*descriptor)->a = 0;
    (*descriptor)->b = 0;
    free((void*)*descriptor);
    *pointerToMyOwnStructPointer = NULL;

    return result;  
}
Mika
  • 11
  • 1
  • 3
0

Answer to your new question.

You should always cast malloc in c++ and never cast malloc in c, as already stated by Shreevardhan and Sourav Ghosh.

All of your void* are in fact myIntegers_t*, so make them myIntegers_t*. It both benefits the reader, but also makes it possible for the compiler to spot errors.

All of your void** are in fact myIntegers_t**. See above.

Unless the contents of a and b is secret, you don't have to make them 0 before freeing. And free doesn't need its argument to be cast to a void*, since void* are compatible with all pointers. It only needs the pointer to have been made by malloc, calloc or realloc.

I don't think the extra descriptor helps anything, except perhaps readability, but then just rename pointerToMyOwnStructPointer.