Skip to content

fix(copyfile): inline linux/version.h#12901

Merged
Alizter merged 1 commit intoocaml:mainfrom
Alizter:push-vluuszlokrzk
Dec 9, 2025
Merged

fix(copyfile): inline linux/version.h#12901
Alizter merged 1 commit intoocaml:mainfrom
Alizter:push-vluuszlokrzk

Conversation

@Alizter
Copy link
Copy Markdown
Collaborator

@Alizter Alizter commented Dec 9, 2025

The kernel headers cannot be expected to be available on every linux distribution. The version header can therefore be inlined easily.

In the kernel, the generated file looks like:

define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) +  \
	((c) > 255 ? 255 : (c)))';

but we don't really need the extra check on c.

fix #12896.

The kernel headers cannot be expected to be available on every linux
distribution. The version header can therefore be inlined easily.

In the kernel, the generated file looks like:

```
define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) +  \
	((c) > 255 ? 255 : (c)))';
```

but we don't really need the extra check on `c`.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter mentioned this pull request Dec 9, 2025
43 tasks
@Alizter Alizter requested a review from nojb December 9, 2025 11:18
Copy link
Copy Markdown
Collaborator

@nojb nojb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Duplicating the exact definition from the kernel headers is not necessary; any definition that allows comparing with < would be sufficient, but using the same one as in the kernel headers works, of course.

@Alizter
Copy link
Copy Markdown
Collaborator Author

Alizter commented Dec 9, 2025

Duplicating the exact definition from the kernel headers is not necessary; any definition that allows comparing with < would be sufficient, but using the same one as in the kernel headers works, of course.

Just to be clear, because I think I worded it a bit confusingly, this isn't exactly the same as the kernel version. Only the c part differs.

@nojb
Copy link
Copy Markdown
Collaborator

nojb commented Dec 9, 2025

Duplicating the exact definition from the kernel headers is not necessary; any definition that allows comparing with < would be sufficient, but using the same one as in the kernel headers works, of course.

Just to be clear, because I think I worded it a bit confusingly, this isn't exactly the same as the kernel version. Only the c part differs.

Right, thanks, I had missed the point of your comment. The extra check in the kernel is to avoid overflow (see https://lwn.net/Articles/845120/). Not including it has the effect that the calculated version is sometimes "higher" than one would expect (eg KERNEL_VERSION(5,19,256) coincides with KERNEL_VERSION(5,20,0)), but indeed, this does not make any difference in our situation.

@Alizter Alizter merged commit f83783c into ocaml:main Dec 9, 2025
29 checks passed
@Alizter Alizter deleted the push-vluuszlokrzk branch December 9, 2025 12:18
@Alizter
Copy link
Copy Markdown
Collaborator Author

Alizter commented Dec 9, 2025

Doesn't need a changelog since we are fixing a regression since 3.20.

@shonfeder shonfeder mentioned this pull request Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copyfile stubs fail on alpine linux

2 participants