5

I'm writing a header, timedate.h, which begins as follows:

#ifndef _TIMEDATE_H_
#define _TIMEDATE_H_

int timetounixtime(int year, int month, int day, int hour, int minute, int second)
{
  struct tm *time;
  time->tm_year = year;
  time->tm_mon = month;
  time->tm_mday = day;
  time->tm_hour = hour;
  time->tm_min = minute;
  time->tm_sec = second;
  return mktime(time);
}

/*...*/

#endif

And is then included in one of my main .c files as follows:

#include <stdio.h>
#include <string.h>
#include <sys/time.h>
#include "timedate.h"

int main(int argv, char **argc)
{
/*...*/
}

It seems to me that this should work since time.h is included in the main code before timedate.h is called. However, when I make, I get the following errors:

XXXXXXXXXX$ make
gcc file2nav.c -o file2nav
In file included from file2nav.c:4:0:
timedate.h: In function ‘timetounixtime’:
timedate.h:10:7: error: dereferencing pointer to incomplete type
timedate.h:11:7: error: dereferencing pointer to incomplete type
timedate.h:12:7: error: dereferencing pointer to incomplete type
timedate.h:13:7: error: dereferencing pointer to incomplete type
timedate.h:14:7: error: dereferencing pointer to incomplete type
timedate.h:15:7: error: dereferencing pointer to incomplete type

Can you help me understand what's going on? I note that if I #include <time.h> in timedate.h, the error goes away...But why? It's already included in file2nav.c.

Frank Harris
  • 305
  • 1
  • 6
  • 16
  • 1
    It's not a backtracking compiler. Just include the files you actually need in your header. Relying on include order is a horrible idea. – Ed S. Jun 11 '13 at 17:23
  • Header files are for function declaration not function definition. Just think what would happen, if you included that file twice. – mohit Jun 11 '13 at 17:27
  • @EdS., I'm confused about why you have to #include in the header file, but you don't have to #include or any other of the usual standard library headers? – Frank Harris Jun 11 '13 at 17:34
  • `timetounixtime` should better return `time_t`. – alk Jun 11 '13 at 17:39
  • @FrankHarris: You have to have the definition (or at least a forward declaration in some cases) of this things that you are actually using. They must be in order. The compiler will not overlook and error in the hops that it will be rectified in the future. If you create a `tm` or dereference a `tm*` in your code you must have the definition of `tm` beforehand. – Ed S. Jun 11 '13 at 17:59

6 Answers6

11

In your file timedate.h you use

struct tm *time;

but struct tm is not defined yet. You need to include the header #include <time.h>.

A second problem in your code is that you're using an uninitialized pointer time. You can use a local variable:

struct tm time;
time.tm_year = year;

or malloc a pointer (remember to free):

struct tm* time = malloc(sizeof(struct tm));

A better practice, as Ryan points out, is to declare functions in .h and define them in .c:

/* timedate.h */
#ifndef _TIMEDATE_H_
#define _TIMEDATE_H_

int timetounixtime(int year, int month, int day, int hour, int minute, int second);

#endif

and

/* timedate.c */
#include "timedate.h"
#include <time.h>

int timetounixtime(int year, int month, int day, int hour, int minute, int second)
{
  struct tm time;
  time.tm_year = year;
  time.tm_mon = month;
  time.tm_mday = day;
  time.tm_hour = hour;
  time.tm_min = minute;
  time.tm_sec = second;
  return mktime(time);
}

You need to include all header files to make your program compile. C++ Header order suggests one possible order:

  • corresponded header file
  • necessary project headers
  • 3rd party libraries headers
  • standard libraries headers
  • system headers

In this order you will not miss any of your header files that forgot to include libraries by their own. (Thank Josh for this point).

Community
  • 1
  • 1
Yang
  • 7,712
  • 9
  • 48
  • 65
  • I guess he thought it would be defined by having `` included in his main file. – Kninnug Jun 11 '13 at 17:24
  • @Kninnug, yeah, that's what I was thinking, and I don't yet understand why that's not the case. My understanding is that the preprocessor inserts the text of time.h in the .c file, prior to the insertion of the text of my header file, so when the compiler gets to that part of the code, it should have already read the definition of tm. – Frank Harris Jun 11 '13 at 17:28
  • @Yang also, I made the change to a local variable. Not sure what I was doing there. I need to go get a coffee maker for my office. – Frank Harris Jun 11 '13 at 17:30
  • 2
    @FrankHarris: `` and `` are different files. – jxh Jun 11 '13 at 17:30
  • 1
    A minor suggestion, if you include user headers first and system headers last in your implementation, then you are less likely to have order-dependent behavior. – Josh Petitt Jun 11 '13 at 18:41
3

You need to #include <time.h> in you timedate.h file because the function timetounixtime uses a struct declared in it. That function needs to know what a struct tm is, and it doesn't unless you include time.h. There are several other problems here though.

You need to allocate space for your tm struct as such:

struct tm *time = malloc(sizeof *time);

but since you're only using it in this one function, you should just be doing

struct tm time;

otherwise you're using invalid memory when you start assigning.

This header file should be separated into two files, additionally.

/* timedate.h */
#ifndef _TIMEDATE_H_
#define _TIMEDATE_H_

int timetounixtime(int year, int month, int day, int hour, int minute, int second);

#endif

and

/* timedate.c */
#include "timedate.h"

int timetounixtime(int year, int month, int day, int hour, int minute, int second)
{
  struct tm time;
  time.tm_year = year;
  time.tm_mon = month;
  time.tm_mday = day;
  time.tm_hour = hour;
  time.tm_min = minute;
  time.tm_sec = second;
  return mktime(time);
}

I suggest in the future you compile with gcc -Wall. You'll get useful warnings like this one:

timedate.h:15:3: warning: implicit declaration of function 'mktime' [-Wimplicit-function-declaration]

^which means you are calling the mktime function without ever declaring it, another symptom of forgetting time.h

Ryan Haining
  • 35,360
  • 15
  • 114
  • 174
3

You include the wrong header, it should be <time.h>, not <sys/time.h>.

<sys/time.h> probably simply doesn't define the struct you are trying to use.

sth
  • 222,467
  • 53
  • 283
  • 367
2

Do not include sys/time.h but time.h.

alk
  • 69,737
  • 10
  • 105
  • 255
1

time also happens to be a system call. I'll suggest changing the variable name time to something else so as not to cause a conflict with the system call.

unxnut
  • 8,509
  • 3
  • 27
  • 41
0

You need to include the time.h header from the header file because it won't know what the struct tm and mktime symbols are.

You would need forward declarations of those symbols in order for them to link properly when the time.h header eventually gets included from your source file.

bojangler
  • 544
  • 2
  • 6