OSDev.org

Posted: **Sun Sep 12, 2021 6:10 pm**

Hello again,
I've been working on IPC and got stuck with a compiler error...

I'm trying to be able to use the ipc_response type in both kernel/ipc.h and kernel/task.h (as well as any files that include these headers).
It is defined in kernel/ipc.h which is included by task.h, but I keep getting the following error:

Code: Select all

include/kernel/task.h:21:1: error: unknown type name 'ipc_response'
 ipc_response *get_ipc_responses(uint32_t pid);
 ^~~~~~~~~~~~

Except kernel/task.h INCLUDES kernel/ipc.h, so it should be defined!

I've also tried moving the type definition to task.h, but it just shows a similar error for ipc.h instead.
I realize the headers are circularly dependent (ipc.h relies on a few functions from task.h, and vice versa), but since there's header guards it shouldn't matter.

I've also tried adding the definition to BOTH headers, but it says

Code: Select all

tritium-os/sysroot/usr/include/kernel/task.h:15:27: error: conflicting types for 'ipc_response'
 } __attribute__((packed)) ipc_response;
                           ^~~~~~~~~~~~

How do I resolve this?

Code in following post.

Posted: **Sun Sep 12, 2021 6:11 pm**

kernel/ipc.h:

Code: Select all

#ifndef _KERNEL_IPC_H
#define _KERNEL_IPC_H

#include <kernel/stdio.h>
#include <kernel/task.h>

#define IPC_MAX_RESPONSES (4096/sizeof(void*))

typedef struct {
	void *phys_data[IPC_MAX_RESPONSES];
} __attribute__((packed)) ipc_response;

bool init_ipc(void);
bool register_ipc_port(uint16_t port);
bool deregister_ipc_port(uint16_t port, bool override);
void deregister_ipc_ports_pid(uint32_t pid);
uint8_t transfer_ipc(uint16_t port, void *data, size_t size);
size_t receive_ipc_size(uint16_t port, bool override);
uint8_t receive_ipc(uint16_t port, void *data);
bool verify_ipc_port_ownership(uint32_t port);
bool transfer_avail(uint32_t pid, uint32_t port);
void recover_response_pages(ipc_response *ipc_responses);

#endif

kernel/task.h:

Code: Select all

#ifndef _KERNEL_TASK_H
#define _KERNEL_TASK_H

#include <fs/file.h>
#include <kernel/tss.h>
#include <kernel/ksetup.h>
#include <kernel/stdio.h>
#include <kernel/elf.h>
#include <kernel/ipc.h>

void task_switch(tss_entry_t tss);
void create_process(void *prgm,size_t size);
void create_idle_process(void *prgm, size_t size);
void init_tasking(uint32_t num_pages);
void start_program(char *name);
void exit_program(int retval);
void kill_program(uint32_t pid, uint32_t retval);
uint32_t fork_process(void);
void waitipc(uint32_t port);
ipc_response *get_ipc_responses(uint32_t pid);

#endif

Posted: **Sun Sep 12, 2021 6:34 pm**

foliagecanine wrote:I realize the headers are circularly dependent

No they're not.

The corresponding .c files might depend on functions in both headers, but ipc.h doesn't depend on anything in task.h.

Also, you probably don't need __attribute__((packed)).

Posted: **Sun Sep 12, 2021 7:00 pm**

Octocontrabass wrote:No they're not.

You're probably right. I was just pointing out that ipc.h includes task.h and vice versa.

Octocontrabass wrote:Also, you probably don't need __attribute__((packed)).

This structure is supposed to use 4096 bytes or less (to fit in a single page) and have as many elements in the phys_data array as possible.
I realize __attribute__((packed)) doesn't really matter if it is a single array inside of the struct, but I am probably going to add more items to the struct (and change the math for IPC_MAX_RESPONSES) later.
Unless there's a better way to do this...?

Posted: **Sun Sep 12, 2021 7:17 pm**

foliagecanine wrote:I realize the headers are circularly dependent (ipc.h relies on a few functions from task.h, and vice versa), but since there's header guards it shouldn't matter.

It does matter; it's the reason for your problem.

If you #include kernel/task.h:

kernel/task.h includes kernel/ipc.h before it has defined ipc_response;
kernel/ipc.h includes kernel/task.h but the header guard (_KERNEL_TASK_H) is already defined, so it has no effect;
kerne/ipc.h then refers to the ipc_response type, which hasn't yet been defined.

You should probably find a way avoid the circular include.

Posted: **Sun Sep 12, 2021 9:53 pm**

You can avoid circular includes by using incomplete types. For that, you need to use tag names. So you change your task.h to no longer include ipc.h and instead contain:

Code: Select all

struct ipc_response;

And all other definitions in that file use "struct ipc_response" as type, instead of just "ipc_response". Then in ipc.h, you define

Code: Select all

typedef struct ipc_response {
/* whatever */
} ipc_response;

Or, you know, you can just delete the inclusion of task.h in ipc.h. Then you don't need any further changes.

Posted: **Sun Sep 12, 2021 10:07 pm**

davmac314 wrote:kernel/task.h includes kernel/ipc.h before it has defined ipc_response;

How does it include it before ipc_response is defined?

What I believe should happen is this:

When compiling something that includes task.h:
1. _KERNEL_TASK_H is defined
2. Other includes happen, then ipc.h is included (essentially copy-pasted) into that spot.
3. _KERNEL_IPC_H is defined
4. Other includes happen, then task.h is included again, but doesn't get evaluated because _KERNEL_TASK_H is defined
5. Code is compiled, including the ipc_response struct definition

When compiling something that includes ipc.h:
1. _KERNEL_IPC_H is defined
2. Other includes happen, then task.h is included into that spot.
3. _KERNEL_TASK_H is defined
4. Other includes happen, then ipc.h is included again, but doesn't get evaluated because _KERNEL_IPC_H is defined
5. Code is compiled, including the ipc_response struct definition

Am I missing something?

nullplan wrote:You can avoid circular includes by using incomplete types.

Problem is I also need the function prototypes from both files, not just the struct definition.
And even if I comment out the task.h include in ipc.h it still doesn't work.

Posted: **Sun Sep 12, 2021 11:15 pm**

Code: Select all

#ifndef IPC_H
#define IPC_H
#include "task.h"
typedef struct {
	int a;
} ipc_response;
#endif

Code: Select all

#ifndef TASK_H
#define TASK_H
#include "ipc.h"
ipc_response *func();
#endif

Code: Select all

#include "ipc.h"
int main(){return 0;}

Code: Select all

In file included from ipc.h:4,
                 from 1.c:3:
task.h:6:1: error: unknown type name ‘ipc_response’
    6 | ipc_response *func();
      | ^~~~~~~~~~~~

- #include for ipc.h
- IPC_H is defined.
- #include for task.h
- TASK_H is defined.
- ipc.h include is attempted again but doesn't succeed because IPC_H is already defined.
- ipc_response is not found when processing the func() declaration.
- #endif for task.h
- ipc_response defined here.
- #endif for ipc.h

The problem also goes away, at least in this test case, if the typedef of the struct is removed and the struct name is used as is.

Edit: Add some details to the steps.

Posted: **Mon Sep 13, 2021 12:34 pm**

linuxyne wrote:- #include for ipc.h
- IPC_H is defined.
- #include for task.h
- TASK_H is defined.
- ipc.h include is attempted again but doesn't succeed because IPC_H is already defined.
- ipc_response is not found when processing the func() declaration.
- #endif for task.h
- ipc_response defined here.
- #endif for ipc.h

Okay, that makes more sense.
It seems like I should be able to typedef ipc_response before I #include task.h inside ipc.h.
I can't test this hypothesis right now but I will when I can.

Posted: **Mon Sep 13, 2021 5:33 pm**

Sorry, yeah, I messed up my description of the problem; got the headers mixed up. It sounds like you understand the problem now anyway.

Posted: **Mon Sep 13, 2021 7:29 pm**

I couldn't get it to work even after moving ipc_response above the #include task.h

I eventually decided just to tag the struct and typedef it in each file separately.

Posted: **Tue Sep 14, 2021 10:17 am**

foliagecanine wrote:I couldn't get it to work even after moving ipc_response above the #include task.h

I eventually decided just to tag the struct and typedef it in each file separately.

Might be a good opportunity to highlight a useful tool that helps me avoid this issue completely:

https://fossil-scm.org/home/doc/trunk/s ... aders.html

The tool is from the same mind as SQLite, and Fossil SCM (which I also use), and it means I barely ever have to write a header file. The tool instead scans all my C source files, and generates a specific header file for each C source file, such that all the declarations required in that C source file are included in the header.

So, for example, my:

kernel/thread.c

Includes just "thread.h", which is generated with all the defines from the other source files required in thread.c.

It probably also speeds up compiles, as each source has a minimal set of declarations to compile.

OSDev.org

Struct in header won't compile...

Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...

Re: Struct in header won't compile...