| 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
| ||
|
Message-ID: <15af6841-17f5-2bf0-cf57-597ce3cf9b6f@intel.com>
Date: Thu, 19 Oct 2023 14:34:53 +0200
From: Karolina Stolarek <karolina.stolarek@...el.com>
To: Calvince Otieno <calvncce@...il.com>
CC: <outreachy@...ts.linux.dev>, <gustavo@...eddedor.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>,
<linux-staging@...ts.linux.dev>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] staging: vc04_services: remove empty functions
On 19.10.2023 13:21, Calvince Otieno wrote:
> On Thu, Oct 19, 2023 at 1:55 PM Karolina Stolarek
> <karolina.stolarek@...el.com> wrote:
>>
>> On 19.10.2023 11:46, Calvince Otieno wrote:
>>> The functions vchiq_debugfs_init(), vchiq_debugfs_deinit(),
>>> vchiq_debugfs_add_instance(), and vchiq_debugfs_remove_instance()
>>> are declared and defined but contains no code or statements.
>>> They do nothing.
>>>
>>> Signed-off-by: Calvince Otieno <calvncce@...il.com>
>>> ---
>>> .../interface/vchiq_arm/vchiq_arm.c | 3 ---
>>> .../interface/vchiq_arm/vchiq_debugfs.c | 16 ----------------
>>> .../interface/vchiq_arm/vchiq_debugfs.h | 8 --------
>>> .../interface/vchiq_arm/vchiq_dev.c | 4 ----
>>> 4 files changed, 31 deletions(-)
>>>
>>> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>>> index eef9c8c06e66..b802d1ecc8f6 100644
>>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>>> @@ -1806,8 +1806,6 @@ static int vchiq_probe(struct platform_device *pdev)
>>> if (err)
>>> goto failed_platform_init;
>>>
>>> - vchiq_debugfs_init();
>>> -
>>> vchiq_log_info(vchiq_arm_log_level,
>>> "vchiq: platform initialised - version %d (min %d)",
>>> VCHIQ_VERSION, VCHIQ_VERSION_MIN);
>>> @@ -1838,7 +1836,6 @@ static void vchiq_remove(struct platform_device *pdev)
>>> {
>>> vchiq_device_unregister(bcm2835_audio);
>>> vchiq_device_unregister(bcm2835_camera);
>>> - vchiq_debugfs_deinit();
>>> vchiq_deregister_chrdev();
>>> }
>>>
>>> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
>>> index dc667afd1f8c..7e45076e5ebd 100644
>>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
>>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
>>> @@ -228,20 +228,4 @@ void vchiq_debugfs_deinit(void)
>>>
>>> #else /* CONFIG_DEBUG_FS */
>> We need to have these definitions, so a kernel with no CONFIG_DEBUG_FS
>> selected builds. Have you experimented with this config with your
>> changes applied?
>>
>> All the best,
>> Karolina
>>
>>>
>>> -void vchiq_debugfs_init(void)
>>> -{
>>> -}
>>> -
>>> -void vchiq_debugfs_deinit(void)
>>> -{
>>> -}
>>> -
>>> -void vchiq_debugfs_add_instance(struct vchiq_instance *instance)
>>> -{
>>> -}
>>> -
>>> -void vchiq_debugfs_remove_instance(struct vchiq_instance *instance)
>>> -{
>>> -}
>>> -
>>> #endif /* CONFIG_DEBUG_FS */
>>> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h
>>> index e9bf055a4ca9..76424473c32b 100644
>>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h
>>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h
>>> @@ -10,12 +10,4 @@ struct vchiq_debugfs_node {
>>> struct dentry *dentry;
>>> };
>>>
>>> -void vchiq_debugfs_init(void);
>>> -
>>> -void vchiq_debugfs_deinit(void);
>>> -
>>> -void vchiq_debugfs_add_instance(struct vchiq_instance *instance);
>>> -
>>> -void vchiq_debugfs_remove_instance(struct vchiq_instance *instance);
>>> -
>>> #endif /* VCHIQ_DEBUGFS_H */
>>> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>>> index 841e1a535642..953f39f537a8 100644
>>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>>> @@ -1185,8 +1185,6 @@ static int vchiq_open(struct inode *inode, struct file *file)
>>> instance->state = state;
>>> instance->pid = current->tgid;
>>>
>>> - vchiq_debugfs_add_instance(instance);
>>> -
>>> init_completion(&instance->insert_event);
>>> init_completion(&instance->remove_event);
>>> mutex_init(&instance->completion_mutex);
>>> @@ -1297,8 +1295,6 @@ static int vchiq_release(struct inode *inode, struct file *file)
>>>
>>> free_bulk_waiter(instance);
>>>
>>> - vchiq_debugfs_remove_instance(instance);
>>> -
>>> kfree(instance);
>>> file->private_data = NULL;
>>>
>
> Could you kindly clarify what these definitions are doing.
> At the moment they do absolutely nothing, I might be wrong
These functions have different definitions, depending on the value of
DEBUG_FS[1]. If this symbol is defined, these functions create entries
in /sys/kernel/debug, which are useful for kernel developers. Please
mind that these functions are defined twice in that file. For example:
https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c#L206
https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c#L231
And like I said before, we have to also define empty functions, so no
matter if that config option is selected or not, the kernel can be built.
All the best,
Karolina
---------------------------------
[1] - https://elixir.bootlin.com/linux/latest/source/lib/Kconfig.debug#L626
Powered by blists - more mailing lists