Expression must have pointer to struct or union error

Question

t variable is coming up with an error from assigntime function onwards saying it must have a pointer to a struct or union type. pointers are my weakness, if anyone could explain, not just give me the answer, what i need to do to fix this that would be most helpful! cheers.

//MY TIME C FILE
#include "my_time.h"
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

struct my_time_int 
{
    int hour;
    int minute;
    int second;
};

void init_my_time(my_time *t)
{

    t=(my_time)malloc(sizeof(struct init_my_time*)); 
}

/*
 * Alter hour, minute, and second
 * Param h new value for hour
 * Param m new value for minute
 * Param s new value for second
*/
void assignTime(my_time *t, int h, int m, int s) 
{
    t->hour = h;
    t->minute = m;
    t->second = s;
}
//FOLLOWING CODE T VARIABLE HAS RED UNDERLINE ERROR SAYING EXPRESSION MUST HAVE POINTER TO STRUCT OR UNION>
char *toString(my_time t)
{ 
    char *r = (char *)malloc(12 * sizeof(char));

    if (t.hour >= 12) {
    if (t.hour == 12)
        sprintf(r, "%02d:%02d:%02d PM", 12, t.minute, t.second);
    else
        sprintf(r, "%02d:%02d:%02d PM", t.hour - 12, t.minute, t.second);
    }
    else {
        if (t.hour == 0)
        sprintf(r, "%02d:%02d:%02d AM", 12, t.minute, t.second);
        else
        sprintf(r, "%02d:%02d:%02d AM", t.hour, t.minute, t.second);
    }

    return r;

}

/*
 * Find printable form of time in 24 hour mode
 * Return String form of time in 24 hour mode for printing etc.
*/
char *toMilString(my_time t)
{ 
    char *s = (char *)malloc(9 * sizeof(char));

    sprintf(s, "%02d:%02d:%02d", t.hour, t.minute, t.second);
    return s;
}

/*
 * Find number of seconds elapsed since midnight
 * Return number of seconds elapsed since midnight as int
*/
int secsSinceMidnight(my_time t)
{
return t.second + (60 * t.minute) + (60 * 60 * t.hour);
}

Header File Here:

#include <stdbool.h>

struct my_time_int;
typedef struct my_time_int *my_time;

void init_my_time(my_time *t);
void assignTime(my_time *t, int h, int m, int s);
void addTime(my_time t, double s);
char *toString(my_time t);
char *toMilString(my_time t);
bool equals(my_time this, my_time that);
bool my_timeIncHour(my_time *t);
bool my_timeIncMinute(my_time *t);
bool my_timeIncSecond(my_time *t);

Don't obfuscate your code by putting a pointer type into a `typedef`. When you use the `->` on variables of type `my_time*` you fall into your own trap. Also, don't obfuscate `malloc` by casting its return type. — Jens Gustedt, Apr 26 '15 at 07:08
See [Is it a good idea to typedef pointers?](http://stackoverflow.com/questions/750178/is-it-a-good-idea-to-typedef-pointers) for a detailed discussion of @Jens's first point, and [Do I cast the result of `malloc()`?](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) for a discussion of his second point. — Jonathan Leffler, Apr 26 '15 at 07:14

Giova · Answer 1 · 2015-04-26T09:01:29.987

There are a couple of errors in your code.

Primarily the use of pointers it's not correct in respect to the desired outcome. In the header you have the line:

typedef struct my_time_int *my_time;

which effectively declares my_timebeing a pointer type to the struct my_time_int. But in your function's prototypes (and definitions too) you use a pointer to my_time as argument: my_time* t. In fact here you are using a pointer to a pointer to the struct my_time_int.

So when you try to assign to tusing the deference arrow operator -> you make a mistake, because in fact you are assigning to a pointer to a pointer to a struct not to a plain pointer to a struct as you wish.

You should also avoid using the . operator on variables of type my_time because they are in facts pointers. You should use instead the arrow ->operator on them.

Here the proposed solution:

//MY TIME C FILE
#include "prova.h"
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

struct my_time_int
{
    int hour;
    int minute;
    int second;
};

//void init_my_time(my_time *t)
my_time init_my_time()
{
    //t=(my_time)malloc(sizeof(struct init_my_time*));
    return (my_time)malloc(sizeof(struct my_time_int));
}

/*
 * Alter hour, minute, and second
 * Param h new value for hour
 * Param m new value for minute
 * Param s new value for second
*/
//void assignTime(my_time *t, int h, int m, int s) 
void assignTime(my_time t, int h, int m, int s)
{
    t->hour = h;
    t->minute = m;
    t->second = s;
} 

//FOLLOWING CODE T VARIABLE HAS RED UNDERLINE ERROR SAYING EXPRESSION MUST             HAVE POINTER TO STRUCT OR UNION>
char *toString(my_time t)
{ 
char *r = (char *)malloc(12 * sizeof(char));

//if (t.hour >= 12) {
if(t->hour >= 12){
//if (t.hour == 12)
if(t->hour == 12)
    //sprintf(r, "%02d:%02d:%02d PM", 12, t.minute, t.second);
    sprintf(r, "%02d:%02d:%02d PM", 12, t->minute, t->second);
else
    //sprintf(r, "%02d:%02d:%02d PM", t.hour - 12, t.minute, t.second);
    sprintf(r, "%02d:%02d:%02d PM", t->hour - 12, t->minute, t->second);
}
else {
    //if (t.hour == 0)
    if (t->hour == 0)
    //sprintf(r, "%02d:%02d:%02d AM", 12, t.minute, t.second);
    sprintf(r, "%02d:%02d:%02d AM", 12, t->minute, t->second);
    else
    //sprintf(r, "%02d:%02d:%02d AM", t.hour, t.minute, t.second);
    sprintf(r, "%02d:%02d:%02d AM", t->hour, t->minute, t->second);
}

return r;

}

/*
 * Find printable form of time in 24 hour mode
 * Return String form of time in 24 hour mode for printing etc.
*/
char *toMilString(my_time t)
{ 
    char *s = (char *)malloc(9 * sizeof(char));

    //sprintf(s, "%02d:%02d:%02d", t.hour, t.minute, t.second);    
    sprintf(s, "%02d:%02d:%02d", t->hour, t->minute, t->second);
    return s;
}

/*
 * Find number of seconds elapsed since midnight
 * Return number of seconds elapsed since midnight as int
*/
int secsSinceMidnight(my_time t)
{
//return t.second + (60 * t.minute) + (60 * 60 * t.hour);
return t->second + (60 * t->minute) + (60 * 60 * t->hour);
}

And header too:

#include <stdbool.h>

struct my_time_int;
typedef struct my_time_int *my_time;

//void init_my_time(my_time *t);
my_time init_my_time();
//void assignTime(my_time *t, int h, int m, int s);
void assignTime(my_time t, int h, int m, int s);

//and son on removing the unnecessary pointer types
void addTime(my_time t, double s);
char *toString(my_time t);
char *toMilString(my_time t);
bool equals(my_time this, my_time that);
bool my_timeIncHour(my_time t);
bool my_timeIncMinute(my_time t);
bool my_timeIncSecond(my_time t);

As you can se in the commented code there are the previous erroneous declarations and definitions.

EDIT

As pointed in the comments, the init_my_time, as defined, leaks memory because allocates a pointer that it doesn't return to the caller. The right thing to do here is to allocate the memory and return the pointer to that memory to the caller. This requires changing the declaration and definition of init_my_time as already done above in the code.

+1, but I'd really prefer a version where the `*` just would be removed from the `typedef`. Also the cast for the return of `malloc` should be avoided. — Jens Gustedt, Apr 26 '15 at 07:13
It would be also better for the sake of readability, because if you read only my_time.c you don't get the point that my_time type is a pointer type. So I absolutely agree with you! — Giova, Apr 26 '15 at 07:18
The proposed replacement initialization function doesn't relay the pointer to the allocated memory back to the calling function, which means it leaks. — Jonathan Leffler, Apr 26 '15 at 07:46
wow thankyou so much that makes it much easier to understand. sorry for being an embarrassment to the programming community — Jonno Williams, Apr 26 '15 at 08:02
i have made amendments however still getting error saying expression must have pointer to a struct, ill post the rest of my code for time if you wish to correct me further lol, thankyou again — Jonno Williams, Apr 26 '15 at 08:10
ive just noticed my header and c files in visual studios have a star on the end eg " my_time.h* " is the star impacting the code at all? never seen this before — Jonno Williams, Apr 26 '15 at 08:35
The star indicates that you have unsaved modifications on those files. If you save it disappear. Uh, don't forget to chose the answer! — Giova, Apr 26 '15 at 08:46

Benjy Kessler · Answer 2 · 2015-04-26T06:47:50.987

0

Try malloc(sizeof(struct my_time_int)); instead of malloc(sizeof(struct init_my_time*)); You are allocating memory for a pointer to init_my_time instead of for my_time.

t is a pointer to a pointer to a my_time_int that means you need to allocate enough memory to store a my_time_int object. The pointer itself is stored on the stack so you don't need to allocate memory for it.

void init_my_time(my_time *t)
{
    *t=(my_time)malloc(sizeof(struct my_time_int)); 
}

edited Apr 26 '15 at 06:47

answered Apr 26 '15 at 06:30

Benjy Kessler

7,356
6
41
69

Maybe also address why that allocation never makes it out of `init_my_time`. Remember, `t` is just a local var passed by value, so we get no allocation back to the caller and a fine memory leak. And *generally* `malloc(sizeof *ptr)` is preferable, as it pins the allocation to the target pointer type (and saves keystrokes as a bonus). – WhozCraig Apr 26 '15 at 06:39
so i need t to point to my_time and my_time to point to my_time_int? since thats where the struct is? thanks for you responses guys. i have editted that line to: t=(my_time)malloc(sizeof(struct my_time* )); T variable is still getting an error though in the rest of my code :/ – Jonno Williams Apr 26 '15 at 06:44
I just noticed that you defined my_time to be a pointer to my_time_int. I will edit my response. – Benjy Kessler Apr 26 '15 at 06:47
Yea the lecturer was saying for this task we need to hide our code or something as is normal in the industry using pointers. – Jonno Williams Apr 26 '15 at 06:49
p.s i originally tried that line you just put up however i get an "expression must have arithmetic type" error at the start around the t variable and equals sign. c code is the worst for debugging i swear lol. – Jonno Williams Apr 26 '15 at 06:51
"expression must have arithmetic type" comes up when i move my arrow over the t variable. its underlined in red! removing the " * " removes the error for this line though. – Jonno Williams Apr 26 '15 at 06:54

Expression must have pointer to struct or union error

2 Answers2

Linked