linux-kernel - Re: [PATCH v7 1/6] fieldbus_dev: add Fieldbus Device subsystem.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190122165658.GH26416@windriver.com>
Date:   Tue, 22 Jan 2019 11:56:59 -0500
From:   Paul Gortmaker <paul.gortmaker@...driver.com>
To:     Sven Van Asbroeck <thesven73@...il.com>
CC:     <svendev@...x.com>, <robh+dt@...nel.org>,
        <linus.walleij@...aro.org>, <lee.jones@...aro.org>,
        <mark.rutland@....com>, <afaerber@...e.de>, <treding@...dia.com>,
        <david@...hnology.com>, <noralf@...nnes.org>, <johan@...nel.org>,
        <monstr@...str.eu>, <michal.vokac@...ft.com>, <arnd@...db.de>,
        <gregkh@...uxfoundation.org>, <john.garry@...wei.com>,
        <geert+renesas@...der.be>, <robin.murphy@....com>,
        <sebastien.bourdelin@...oirfairelinux.com>, <icenowy@...c.io>,
        <stuyoder@...il.com>, <maxime.ripard@...tlin.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 1/6] fieldbus_dev: add Fieldbus Device subsystem.

[[PATCH v7 1/6] fieldbus_dev: add Fieldbus Device subsystem.] On 22/01/2019 (Tue 10:21) Sven Van Asbroeck wrote:

> Fieldbus device (client) adapters allow data exchange with a PLC aka.
> "Fieldbus Controller" over a fieldbus (Profinet, FLNet, etc.)
> 
> They are typically used when a Linux device wants to expose itself
> as an actuator, motor, console light, switch, etc. over the fieldbus.
> 
> This framework is designed to provide a generic interface to Fieldbus
> Devices from both the Linux Kernel and the userspace.
> 
> Signed-off-by: Sven Van Asbroeck <TheSven73@...glemail.com>

[...]

> diff --git a/drivers/fieldbus/Kconfig b/drivers/fieldbus/Kconfig
> new file mode 100644
> index 000000000000..5c2bef950d04
> --- /dev/null
> +++ b/drivers/fieldbus/Kconfig
> @@ -0,0 +1,19 @@
> +menuconfig FIELDBUS_DEV
> +	bool "Fieldbus Device Support"

OK, so the core support is NOT tristate, ie not modular, so I think...

> diff --git a/drivers/fieldbus/dev_core.c b/drivers/fieldbus/dev_core.c
> new file mode 100644
> index 000000000000..c816df3201bb
> --- /dev/null
> +++ b/drivers/fieldbus/dev_core.c
> @@ -0,0 +1,348 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Fieldbus Device Driver Core
> + *
> + */
> +
> +#include <linux/fieldbus_dev.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>

...you don't need module.h here

> +#include <linux/device.h>
> +#include <linux/idr.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/poll.h>
> +

[...]

> +
> +static void __exit fieldbus_exit(void)
> +{
> +	unregister_chrdev_region(fieldbus_devt, MAX_FIELDBUSES);
> +	class_unregister(&fieldbus_class);
> +}
> +
> +subsys_initcall(fieldbus_init);
> +module_exit(fieldbus_exit);

...and the module_exit is never called, so fieldbus_exit is dead code
and hence both should be removed.

> +
> +MODULE_AUTHOR("Sven Van Asbroeck <TheSven73@...glemail.com>");
> +MODULE_AUTHOR("Jonathan Stiles <jonathans@...x.com>");
> +MODULE_DESCRIPTION("Fieldbus Device Driver Core");
> +MODULE_LICENSE("GPL v2");

And these are all no-ops for non-modules, so it is recommended that you
instead capture the information at the top of the file in comments.

You can find many similar cleanups for reference with this search:

   git log  --no-merges --oneline --grep='make .* non-modular'

Thanks,
Paul.
--