Re: [PATCH] dm vdo: Fix W=1 allmodconfig build error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/7/25 4:49 AM, John Garry wrote:
On 07/01/2025 01:24, Matthew Sakai wrote:
Hi John,
Hi Matt,

I'm all for getting rid of warnings where we can, but I'm not 
convinced this is the right approach here.
First, we use snprintf here because we really do want to limit the
string to 15 bytes.
Out of curiosity, why is that?
A dm-vdo target creates several threads to do its work. The 
thread_name_prefix is intended to be a short string at the start of the 
thread name that allows users to easily distinguish these dm-vdo threads 
from all others on the system. But it should not be so long that the 
dm-vdo threads cannot be distinguished from each other.
Now that we're talking about it, I think it's probably even better not 
to use the full module name, but instead use a short static string like 
"vdo".
BTW, as I understand, for CONFIG_DM_VDO=m, the module name is "dm_vdo". So why different MODULE_NAME if MODULE is not defined ("dm-vdo")?
That is a historical artifact and it should just get removed. The module 
name should just be a constant string. I could write up a patch to do 
this if you don't want to. It might take me a couple of days though 
because I'm still dealing with some holiday backlog.
Even after we do this, I still think the prefix string should no longer 
rely on the module name.
Making this constant larger doesn't maintain that. This change also increases the size of struct vdo (in vdo.h) by about 50 bytes. While that's not a lot, it does seem unnecessary.
In practice, we know that the our module name is only about 6 bytes, 
and the instance number will never be more than 3, so snprintf will 
never actually truncate anything here. Would checking for a non-zero 
return value suppress the error?
I'm also confused because I don't understand the difference between 
this line and the similar use of snprintf at drivers/md/dm-vdo/funnel- 
workqueue.c:430, which I presume does not trigger a warning?
For that snprintf usage, the compiler just doesn't know the possible 
length of @name, as it is a pointer. That is why it is a good reason to 
use snprintf.
Anyway, I'd be happy with a version that suppresses the warning 
without changing the size of any structures.
Matt Sakai

Thanks,
John

Matt





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux