LinuxKPI: Remove an unused local var fileid from the macro request_module

[zlei]

Summary

The third parameter of function kern_kldload() can be NULL.

No functional change intended.

MFC after: 1 week

[emaste]

sure, there is no need to pass in a temporary variable just to discard the result

[bz]

I don't know if the code path in drm_encoder_slave.c actually uses this; I hit it in iwlwifi yesterday and still have to investigate but having this run from kldloading a module made the system hang in a way that ^T did not work anymore on the console.

[emaste]

I don't know if the code path in drm_encoder_slave.c actually uses this

I don't think the change in this review should have any effect - we just go from discarding the returned fileid to not requesting it at all.

[bz]

In D44583#1017689, @emaste wrote:

I don't know if the code path in drm_encoder_slave.c actually uses this

I don't think the change in this review should have any effect - we just go from discarding the returned fileid to not requesting it at all.

Yes, I understand; hence accepted. I was just curious how (and why) this got spotted given it's been sitting there like this for a decade. If there are (public) consumers I missed it would be good to know as I am still curious as to why it can hang a system (and if it does why has no one noticed it in the past; e.g. what the invariants are).

[emaste]

I was just curious how (and why) this got spotted given it's been sitting there like this for a decade.

Yeah, good point - I'm somewhat curious about that too.

[zlei]

In D44583#1017691, @bz wrote:

In D44583#1017689, @emaste wrote:

I don't know if the code path in drm_encoder_slave.c actually uses this

I don't think the change in this review should have any effect - we just go from discarding the returned fileid to not requesting it at all.

Yes, I understand; hence accepted. I was just curious how (and why) this got spotted given it's been sitting there like this for a decade. If there are (public) consumers I missed it would be good to know as I am still curious as to why it can hang a system (and if it does why has no one noticed it in the past; e.g. what the invariants are).

In D44583#1017692, @emaste wrote:

I was just curious how (and why) this got spotted given it's been sitting there like this for a decade.

Yeah, good point - I'm somewhat curious about that too.

I spotted this while working on D44581 which intends to introduce new kernel-only error code and on D44552 which uses it.

Why it is not spotted and is been sitting there like this for a decade, I'm not sure but I guess kern_kldload() is public and we are passing reference of the on-stack var and compilers (I tested gcc 13.2 and clang 18, with -Wall -Wunused-variable) can NOT infer if kern_kldload () has any side effect thus they hesitate to give a warn message.

As for

why it can hang a system

I have no idea but I'm recently working on kernel linkers so I'm interested with it. May you please share the steps to repeat ?

[zlei]

I spotted this while working on D44581 which intends to introduce new kernel-only error code EUNSDEP and on D44552 which uses it.

As in D44552 kern_kldload() will return the new error. It should not be leaked to the userspace. I was evaluating the impact and @olce pointed out the macro request_module is consuming kern_kldload().