C- an ADT that keeps crashing

Question

I'm trying to create an ADT , first of all, here is the basic information about it:

typedef struct orange_t* Orange;

typedef enum {
JAN, FEB, MAR, APR, MAY, JUN, JUL, AUG, SEP, OCT, NOV, DEC,
} Month;

struct orange_t {

short size;
Month expirationMonth;
char** foodCompanies;
int maxNumberOfFoodCompanies;
int sellingPrice;
};

now im trying to create a function that will "create" new orange like that:

Orange orangeCreate(short size,Month expirationMonth
    ,int maxNumberOfFoodCompanies,int sellingPrice)
 {
   Orange new_orange=(Orange)malloc(sizeof(struct orange_t));
   if(new_orange==NULL)
   {
        return NULL;
    }
   if((sellingPrice<0)||(size>256||size<1)||(maxNumberOfFoodCompanies<0)||(expirationMonth>12)
        ||(expirationMonth<1))
        {
        return NULL;
        }
new_orange->sellingPrice=sellingPrice;
new_orange->maxNumberOfFoodCompanies=maxNumberOfFoodCompanies;
new_orange->expirationMonth=expirationMonth;
new_orange->size=size;
for(int i=0;i<new_orange->maxNumberOfFoodCompanies;i++)
{
    new_orange->foodCompanies[i]=NULL;
}
return new_orange;
}

when I tried to check the function with the simple main():

int main()
{
Orange orange=orangeCreate(3,JAN,10,4);
printf("the size is %d\n",orange->size);
orangeDestroy(orange);
return 0;
}

the program keeps crashing, I guess I didn't change the values in orange as I should, and they might still be NULL. where did I go wrong here?

You are not allocating `foodCompanies` anywhere, but trying to index and assign it. — Eugene Sh., Nov 17 '15 at 16:14
in C, the returned value from `malloc()` and family of functions is type `void *`, which can be assigned to any other pointer. Casting the return type is completely unnecessary and leads to difficulties when debugging and or maintaining the code. The typedef name `Orange` is misleading as it is actually a pointer type. Much better to typedef is as just `struct orange_t` with no pointer, then add the '*' as necessary in the body of the code. — user3629249, Nov 19 '15 at 18:37
an enum starts with 0 (unless specifically defined otherwise) so the check for the `expirationMonth` is not correct, unless the enum is modified so the first entry is: `JAN=1,` — user3629249, Nov 19 '15 at 18:41

score 1 · Answer 1 · answered Nov 17 '15 at 16:16

1

JAN is 0. if expirationMonth<1 you are returning NULL from orangeCreate.

Also: you are not checking the return value of orangeCreate. And you are leaking memory in orangeCreate when you are just returning NULL without freeing the allocated new_orange.

answered Nov 17 '15 at 16:16

Werner Henze

16,404
12
44
69

score 1 · Answer 2 · answered Nov 17 '15 at 16:16

The problem is that you are not allocating memory for char** foodCompanies;

While allocating memory here

Orange new_orange=malloc(sizeof(struct orange_t));

You have allocated space for the pointer variable char** foodCompanies;, but have not allocated space where it would point to.

score 1 · Answer 3 · edited May 23 '17 at 11:59

You have to allocate some memory and store the address of it to new_orange->foodCompanies before accessing it.

Possible fix:

Orange orangeCreate(short size,Month expirationMonth
    ,int maxNumberOfFoodCompanies,int sellingPrice)
 {
   Orange new_orange=malloc(sizeof(struct orange_t));
   if(new_orange==NULL)
   {
        return NULL;
    }
   if((sellingPrice<0)||(size>256||size<1)||(maxNumberOfFoodCompanies<0)||(expirationMonth>12)
        ||(expirationMonth<1))
        {
        return NULL;
        }
new_orange->sellingPrice=sellingPrice;
new_orange->maxNumberOfFoodCompanies=maxNumberOfFoodCompanies;
new_orange->expirationMonth=expirationMonth;
new_orange->size=size;
new_orange->foodCompanies=malloc(sizeof(char*)*new_orange->maxNumberOfFoodCompanies); /* add this line */
for(int i=0;i<new_orange->maxNumberOfFoodCompanies;i++)
{
    new_orange->foodCompanies[i]=NULL;
}
return new_orange;
}

Note: it is discouraged to cast the result of malloc() in C.
c - Do I cast the result of malloc? - Stack Overflow

C- an ADT that keeps crashing

3 Answers3