Skip to content

Conversation

silvanshade
Copy link
Contributor

While working on axboe/liburing#1468 I noticed that the WaitId op allows specifying infop: *const siginfo_t but this is incorrect because the siginfo_t is mutated.

(There may be other cases like this in some of the ops, I haven't checked everything).

This PR updates the method with the correct mutability and adds a test demonstrating a mutation.

@quininer
Copy link
Member

Good catch, but this change is a breaking change. Should we deprecate WaitId and create a new WaitId2?

@silvanshade
Copy link
Contributor Author

silvanshade commented Sep 25, 2025

Good catch, but this change is a breaking change. Should we deprecate WaitId and create a new WaitId2?

Yeah, the breaking change is unfortunate.

Personally I would be hesitant to create a WaitId2 because I think it could potentially be confusing and who knows, maybe there's a possibility something like that would happen upstream eventually in which case that creates another problem.

If it were me, I would probably just allow the breakage this time since I would be surprised if many people are actually using it (and if they are, their code should be updated).

But other than that, I guess I'd opt for bumping the version.

@quininer
Copy link
Member

Sure. I'll consider a new version. This will be merged in next version.

@silvanshade
Copy link
Contributor Author

A non-breaking alternative you may consider in the meantime would be to add a new ::infop_mut() method and deprecate the current ::infop() with a message.

It seems not great to just leave it as-is because it's a potential source of unsoundness if people do end up using it.

@quininer
Copy link
Member

I don't think it's unsoundness, technically *const and *mut are no different.

There are two ways to use infop_mut instead:

  1. ignore infop, which is a breaking change in behavior
  2. check if infop is null, which incurs the overhead of an instruction

I think deprecate infop and add docs without change behavior is best approach.

@silvanshade
Copy link
Contributor Author

I don't think it's unsoundness, technically *const and *mut are no different.

Locally, yes that is true.

But it is possible the data passed as infop is stored somewhere else in the program and is not expected by semantic analysis to be mutably borrowed. This seems like it could technically result in UB even if it is unlikely to happen given that the program won't work as expected anyway.

@silvanshade
Copy link
Contributor Author

I will go ahead and close this since this is a change that will likely not happen for some time.

@silvanshade silvanshade closed this Oct 5, 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.

2 participants