Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

Guenter Roeck
2019-11-06T14:00:22Z

On 11/6/19 1:11 AM, Chunfeng Yun wrote:
> On Tue, 2019-11-05 at 20:03 -0800, Guenter Roeck wrote:
>> On 11/5/19 7:15 PM, Chunfeng Yun wrote:
>>> Try to avoid using extern global variable, and provide two
>>> functions for the usage cases
>>>
>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>> ---
>>> drivers/usb/common/common.c | 16 ++++++++++++++--
>>> include/linux/usb.h | 5 ++++-
>>> 2 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
>>> index 1433260d99b4..639ee6d243a2 100644
>>> --- a/drivers/usb/common/common.c
>>> +++ b/drivers/usb/common/common.c
>>> @@ -293,8 +293,20 @@ struct device *usb_of_get_companion_dev(struct device *dev)
>>> EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
>>> #endif
>>>
>>> -struct dentry *usb_debug_root;
>>> -EXPORT_SYMBOL_GPL(usb_debug_root);
>>> +static struct dentry *usb_debug_root;
>>> +
>>
>> I don't think it is a good idea to declare this variable static
>> before all its uses are removed. This patch will leave the system
>> in a non-compileable state until all patches of the series have
>> been applied.
> Yes, you are right, I'll fix it, may be split into two patch
>
>>
>> I am curious. What is the rationale for this change ?
>> Did exported variables run out of favor ?
> Personally, I think it's good practice to avoid global varibale,
> it may not a good reason.

Personally I don't think it is a good reason at all. When you are done,
someone else may come in and declare the opposite, re-introduce the global
variable, and drop the new functions as unnecessary.

This is ok for new code, but I think we should leave existing code alone
unless it is broken and needs to get fixed. This is not the case here.

Thanks,
Guenter