Re: [PATCH v2 2/2] module: remove srcversion module attribute

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Incomplete feature removal leaves `sumversion.c` as entirely dead code that is still compiled.
- [Low] The `-a` command-line option was not removed from the `getopt` string, causing silent failures if passed.
- [Low] DRBD modules unconditionally report their version string as `"built-in"` even when dynamically loaded.
--

--- Patch [2]: [PATCH v2 2/2] module: remove srcversion module attribute ---
commit 461ecb5f73545c4d962415cbd1b5ef67b64b2f0a
Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

module: remove srcversion module attribute

This commit removes the srcversion module attribute from the kernel and the
helper module tools. Since module versions are no longer used, the
associated logic is dropped.

> diff --git a/drivers/block/drbd/drbd_buildtag.c b/drivers/block/drbd/drbd_buildtag.c
> index cd0389488f63e..22f1e842f9ced 100644
> --- a/drivers/block/drbd/drbd_buildtag.c
> +++ b/drivers/block/drbd/drbd_buildtag.c
> @@ -10,13 +10,8 @@ const char *drbd_buildtag(void)
>  
>  	static char buildtag[38] = "\0uilt-in";
>  
> -	if (buildtag[0] == 0) {
> -#ifdef MODULE
> -		sprintf(buildtag, "srcversion: %-24s", THIS_MODULE->srcversion);
> -#else
> +	if (buildtag[0] == 0)
>  		buildtag[0] = 'b';
> -#endif
> -	}
>  
>  	return buildtag;
>  }

[Severity: Low]
With the removal of the conditional logic in drbd_buildtag(), will this
unconditionally report the DRBD module version string as "built-in" even
when it is dynamically loaded?

[ ... ]

> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index a7b72a81d2482..2e969be7f414c 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c

[ ... ]

> @@ -2327,9 +2306,6 @@ int main(int argc, char **argv)
>  		case 'o':
>  			dump_write = optarg;
>  			break;
> -		case 'a':
> -			all_versions = true;
> -			break;
>  		case 'T':
>  			files_source = optarg;
>  			break;

[Severity: Low]
Should the 'a' flag also be removed from the getopt() option string earlier
in main()?

scripts/mod/modpost.c:main() {
    ...
	while ((opt = getopt(argc, argv, "ei:MmnT:to:au:WwENd:xb")) != -1) {
    ...
}

If the deprecated -a flag is passed to modpost, getopt considers it valid
and returns 'a'. Since the switch statement no longer has a case for 'a', it
falls through to the default case and silently calls exit(1) without
emitting an invalid option error message.

[ ... ]

> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 2aecb8f25c87e..2c2f1d2b40211 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -126,7 +126,6 @@ struct module {
>  	bool seen;
>  	bool has_init;
>  	bool has_cleanup;
> -	char	     srcversion[25];
>  	// Missing namespace dependencies
>  	struct list_head missing_namespaces;
>  	// Actual imported namespaces

[Severity: Low]
Since the srcversion feature is being removed, should
scripts/mod/sumversion.c also be deleted and removed from modpost-objs in
scripts/mod/Makefile?

The file appears to be dead code now that its only entry point
get_src_version() is never called, though the function declaration still
remains further down in scripts/mod/modpost.h.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-module_ver_remove-v2-0-c9163858f382@linuxfoundation.org?part=2