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)
Sat, Mar 30, 10:19 PM
Unknown Object (File)
Mar 8 2024, 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

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
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 38467
Build 35356: arc lint + arc unit

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
38

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
631

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.