Page MenuHomeFreeBSD

Rename struct device to struct _device
ClosedPublic

Authored by markj on Apr 9 2021, 3:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 8, 7:38 PM
Unknown Object (File)
Jan 30 2024, 6:21 PM
Unknown Object (File)
Jan 30 2024, 6:21 PM
Unknown Object (File)
Jan 30 2024, 6:21 PM
Unknown Object (File)
Jan 17 2024, 10:25 PM
Unknown Object (File)
Dec 21 2023, 5:37 PM
Unknown Object (File)
Dec 20 2023, 7:31 AM
Unknown Object (File)
Dec 19 2023, 6:54 AM

Details

Summary

Right now types.h has device_t as a typedef of struct device *. struct
device is defined in subr_bus.c. This means that subsystems that define
a struct device can easily introduce type confusion. The LinuxKPI does
exactly this and there are a couple of bugs as a result.

The type confusion also causes problems for debuggers, since device_t
can refer to different structures depending on how the debugger tries to
resolve it to a type. It also results in type duplication in the CTF
type graph.

Rename struct device to struct _device to avoid these problems.

Almost all of the kernel refers to devices with device_t. A small
handful of drivers referenced struct device * instead, and I have
separate commits to fix those. This diff contains the main name change
and updates headers that shouldn't depend on types.h.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.Apr 9 2021, 3:47 PM

Update a couple of man9 pages that referenced struct device.

gbe added a subscriber: gbe.

LGTM from manpages

This revision is now accepted and ready to land.Apr 9 2021, 10:48 PM

The root cause here is that when linking different compilation units together, struct foo needs to be globally the same for strict compliance with the C standard.
In the long run, the Linux KPI should define strict linux_kpi_device and #define compatibility from there for the user visible parts of the KPI. I imagine this will be quite hard, in practice, given the nature of the linuxkpi code though.

In the mean time, changing this to _device is fine with me. It resolves the conflict in a way that's adequate.

In D29676#665663, @imp wrote:

The root cause here is that when linking different compilation units together, struct foo needs to be globally the same for strict compliance with the C standard.
In the long run, the Linux KPI should define strict linux_kpi_device and #define compatibility from there for the user visible parts of the KPI. I imagine this will be quite hard, in practice, given the nature of the linuxkpi code though.

"device" is a common identifier so I'm not sure how easy it would be to use #define to fix the problem. I remember that this topic came up with the DRM drivers and there weren't any attractive solutions.

In D29676#665663, @imp wrote:

The root cause here is that when linking different compilation units together, struct foo needs to be globally the same for strict compliance with the C standard.
In the long run, the Linux KPI should define strict linux_kpi_device and #define compatibility from there for the user visible parts of the KPI. I imagine this will be quite hard, in practice, given the nature of the linuxkpi code though.

"device" is a common identifier so I'm not sure how easy it would be to use #define to fix the problem. I remember that this topic came up with the DRM drivers and there weren't any attractive solutions.

True. Which is why _device is good for now, but the issues of namespace collision will continue to vex us in general... I too see issues with a simple #define because it is an imperfect match to c's multiple namespaces...

rpokala added inline comments.
sys/kern/subr_bus.c
118
* The structure is named "_device" instead of "device" to avoid type
* confusion caused by other subsystems defining a (struct device).

For readability.

sys/powerpc/include/bus_dma.h
36

Why not

int bus_dma_tag_set_iommu(bus_dma_tag_t, device_t iommu, void *cookie);

?

sys/sys/pcpu.h
190

Similarly,

device_t pc_device; /* CPU device handle */
sys/sys/systm.h
630

And

void _gone_in_dev(device_t dev, int major, const char *msg);
markj marked 4 inline comments as done.

Address feedback.

This revision now requires review to proceed.Apr 10 2021, 3:56 PM
This revision is now accepted and ready to land.Apr 10 2021, 8:35 PM
This revision was automatically updated to reflect the committed changes.