| Message ID | 20220725223707.14477-1-ematsumiya@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | cifs: rename of several structs and variables | expand |
On 7/25/2022 6:36 PM, Enzo Matsumiya wrote: > Hi all, > > This patch set (v2) renames several cifs.ko data structures, variables, and > functions with the goal to improve readability of the code. > > In summary, what's been done: > - change from CamelCase to snake_case > - try to give more meaning to globals and struct members > - rename status fields (server, ses, tcon), define constants for the > various statuses (4/11 can be shared between those structs, others are > specific to each) > - rename of list_head variables to better represent whether they'are > used as a list element ("head") or a list per se. Also tried to give > more meaning to these, as "rlist", "tlist", "llist" looked confusing > and, sometimes, ambiguous. > - remove redundant prefixes from struct members name, e.g. > tcon_tlink's tl_*, smb_rqst's rq_*, cifs_fattr's cf_*, etc > > No functional changes has been made. > > I know these touch some very old code that older devs are highly used > to, but I see this as an improvement to reading the code for everyone. > > I'll be waiting for your reviews and feedback. Enzo, I think this effort is great. If you combine this modernization with refactoring SMB1 into separate files to make it easier to do away with entirely, I'll be even more supportive. Tom. > Cheers, > > Enzo > > v2: > - remove status typedefs (suggested by Christoph Hellwig) > - define status constants instead, reuse some between different > structs so we don't have to create a different set of statuses > for each cifs struct > > Enzo Matsumiya (10): > cifs: rename xid/mid globals > cifs: rename global counters > cifs: rename "TCP_Server_Info" struct to "cifs_server_info" > cifs: rename cifs{File,Lock,Inode}Info structs and more > cifs: convert server info vars to snake_case > cifs: change status and security types enums to constants > cifs: rename cifsFYI to debug_level > cifs: rename list_head fields > cifs: rename more CamelCase to snake_case > cifs: rename more list_heads, remove redundant prefixes > > fs/cifs/Kconfig | 2 +- > fs/cifs/asn1.c | 4 +- > fs/cifs/cifs_debug.c | 158 ++++----- > fs/cifs/cifs_debug.h | 29 +- > fs/cifs/cifs_spnego.c | 4 +- > fs/cifs/cifs_spnego.h | 2 +- > fs/cifs/cifs_swn.c | 24 +- > fs/cifs/cifs_swn.h | 8 +- > fs/cifs/cifs_unicode.c | 4 +- > fs/cifs/cifs_unicode.h | 2 +- > fs/cifs/cifsacl.c | 22 +- > fs/cifs/cifsencrypt.c | 78 ++--- > fs/cifs/cifsfs.c | 124 +++---- > fs/cifs/cifsglob.h | 694 ++++++++++++++++++++-------------------- > fs/cifs/cifsproto.h | 172 +++++----- > fs/cifs/cifssmb.c | 356 ++++++++++----------- > fs/cifs/connect.c | 574 ++++++++++++++++----------------- > fs/cifs/dfs_cache.c | 178 +++++------ > fs/cifs/dfs_cache.h | 40 +-- > fs/cifs/dir.c | 16 +- > fs/cifs/file.c | 636 ++++++++++++++++++------------------ > fs/cifs/fs_context.c | 8 +- > fs/cifs/fs_context.h | 2 +- > fs/cifs/fscache.c | 18 +- > fs/cifs/fscache.h | 10 +- > fs/cifs/inode.c | 530 +++++++++++++++--------------- > fs/cifs/ioctl.c | 18 +- > fs/cifs/link.c | 26 +- > fs/cifs/misc.c | 185 ++++++----- > fs/cifs/netmisc.c | 4 +- > fs/cifs/ntlmssp.h | 6 +- > fs/cifs/readdir.c | 344 ++++++++++---------- > fs/cifs/sess.c | 142 ++++---- > fs/cifs/smb1ops.c | 182 +++++------ > fs/cifs/smb2file.c | 36 +-- > fs/cifs/smb2inode.c | 136 ++++---- > fs/cifs/smb2maperror.c | 2 +- > fs/cifs/smb2misc.c | 72 ++--- > fs/cifs/smb2ops.c | 555 ++++++++++++++++---------------- > fs/cifs/smb2pdu.c | 596 +++++++++++++++++----------------- > fs/cifs/smb2proto.h | 68 ++-- > fs/cifs/smb2transport.c | 112 +++---- > fs/cifs/smbdirect.c | 28 +- > fs/cifs/smbdirect.h | 16 +- > fs/cifs/transport.c | 236 +++++++------- > fs/cifs/xattr.c | 12 +- > 46 files changed, 3230 insertions(+), 3241 deletions(-) >
On 07/25, Tom Talpey wrote: >On 7/25/2022 6:36 PM, Enzo Matsumiya wrote: >>Hi all, >> >>This patch set (v2) renames several cifs.ko data structures, variables, and >>functions with the goal to improve readability of the code. >> >>In summary, what's been done: >>- change from CamelCase to snake_case >>- try to give more meaning to globals and struct members >>- rename status fields (server, ses, tcon), define constants for the >> various statuses (4/11 can be shared between those structs, others are >> specific to each) >>- rename of list_head variables to better represent whether they'are >> used as a list element ("head") or a list per se. Also tried to give >> more meaning to these, as "rlist", "tlist", "llist" looked confusing >> and, sometimes, ambiguous. >>- remove redundant prefixes from struct members name, e.g. >> tcon_tlink's tl_*, smb_rqst's rq_*, cifs_fattr's cf_*, etc >> >>No functional changes has been made. >> >>I know these touch some very old code that older devs are highly used >>to, but I see this as an improvement to reading the code for everyone. >> >>I'll be waiting for your reviews and feedback. > >Enzo, I think this effort is great. If you combine this modernization >with refactoring SMB1 into separate files to make it easier to do >away with entirely, I'll be even more supportive. Tom, thanks for your feedback! As a matter of fact, I do have a branch with SMB1 code isolated [*] and the module renamed to "SMBFS" to modernize this module, while also keeping it SMB-version-agnostic. I'm sending my changes gradually so I don't have to change a lot of things in case there's a lot of negative feedback. [*] - current discussions: - is "smbfs" the best module name? Steve suggested "smb3", but, again, this ties the module to a specific SMB version - should SMB1 code be isolated as in source-code only or should it be a different object (i.e. only built through kernel config and "disable_legacy_dialects" wiped away)? Or even a separate module? In any case, I'm keeping "cifs" as a module alias for SMB1 code for now, and I think we'll need it that way for some time, but at least the internal migration will be done by then. >Tom. Thanks again, Enzo >>v2: >> - remove status typedefs (suggested by Christoph Hellwig) >> - define status constants instead, reuse some between different >> structs so we don't have to create a different set of statuses >> for each cifs struct >> >>Enzo Matsumiya (10): >> cifs: rename xid/mid globals >> cifs: rename global counters >> cifs: rename "TCP_Server_Info" struct to "cifs_server_info" >> cifs: rename cifs{File,Lock,Inode}Info structs and more >> cifs: convert server info vars to snake_case >> cifs: change status and security types enums to constants >> cifs: rename cifsFYI to debug_level >> cifs: rename list_head fields >> cifs: rename more CamelCase to snake_case >> cifs: rename more list_heads, remove redundant prefixes >> >> fs/cifs/Kconfig | 2 +- >> fs/cifs/asn1.c | 4 +- >> fs/cifs/cifs_debug.c | 158 ++++----- >> fs/cifs/cifs_debug.h | 29 +- >> fs/cifs/cifs_spnego.c | 4 +- >> fs/cifs/cifs_spnego.h | 2 +- >> fs/cifs/cifs_swn.c | 24 +- >> fs/cifs/cifs_swn.h | 8 +- >> fs/cifs/cifs_unicode.c | 4 +- >> fs/cifs/cifs_unicode.h | 2 +- >> fs/cifs/cifsacl.c | 22 +- >> fs/cifs/cifsencrypt.c | 78 ++--- >> fs/cifs/cifsfs.c | 124 +++---- >> fs/cifs/cifsglob.h | 694 ++++++++++++++++++++-------------------- >> fs/cifs/cifsproto.h | 172 +++++----- >> fs/cifs/cifssmb.c | 356 ++++++++++----------- >> fs/cifs/connect.c | 574 ++++++++++++++++----------------- >> fs/cifs/dfs_cache.c | 178 +++++------ >> fs/cifs/dfs_cache.h | 40 +-- >> fs/cifs/dir.c | 16 +- >> fs/cifs/file.c | 636 ++++++++++++++++++------------------ >> fs/cifs/fs_context.c | 8 +- >> fs/cifs/fs_context.h | 2 +- >> fs/cifs/fscache.c | 18 +- >> fs/cifs/fscache.h | 10 +- >> fs/cifs/inode.c | 530 +++++++++++++++--------------- >> fs/cifs/ioctl.c | 18 +- >> fs/cifs/link.c | 26 +- >> fs/cifs/misc.c | 185 ++++++----- >> fs/cifs/netmisc.c | 4 +- >> fs/cifs/ntlmssp.h | 6 +- >> fs/cifs/readdir.c | 344 ++++++++++---------- >> fs/cifs/sess.c | 142 ++++---- >> fs/cifs/smb1ops.c | 182 +++++------ >> fs/cifs/smb2file.c | 36 +-- >> fs/cifs/smb2inode.c | 136 ++++---- >> fs/cifs/smb2maperror.c | 2 +- >> fs/cifs/smb2misc.c | 72 ++--- >> fs/cifs/smb2ops.c | 555 ++++++++++++++++---------------- >> fs/cifs/smb2pdu.c | 596 +++++++++++++++++----------------- >> fs/cifs/smb2proto.h | 68 ++-- >> fs/cifs/smb2transport.c | 112 +++---- >> fs/cifs/smbdirect.c | 28 +- >> fs/cifs/smbdirect.h | 16 +- >> fs/cifs/transport.c | 236 +++++++------- >> fs/cifs/xattr.c | 12 +- >> 46 files changed, 3230 insertions(+), 3241 deletions(-) >>
[ resending this as the other one bounced ] On 07/27, Steve French wrote: >Of the first three patches, I think this one makes the most sense, but >the rename should be "TCP_Server_Info" to something like >"tcp_server_info" (or TCP_server_info) or something that indicates it >is for the transport level info, info related to the network >connection, the socket etc. Noted. >(and not using the word "cifs" unless >related to SMB1) The "cifs" prefix is for the bigger picture (SMB1 isolation and/or module renaming) so I can later on do something like: "sed -i 's/cifs_/<newname>_/g' *.c" >---------- Forwarded message --------- >From: Enzo Matsumiya <ematsumiya@suse.de> >Date: Mon, Jul 25, 2022 at 5:37 PM >Subject: [RFC PATCH v2 03/10] cifs: rename "TCP_Server_Info" struct to >"cifs_server_info" >To: <linux-cifs@vger.kernel.org> >Cc: <smfrench@gmail.com>, <pc@cjr.nz>, <ronniesahlberg@gmail.com>, ><nspmangalore@gmail.com> > > >Rename the TCP_Server_Info struct to "cifs_server_info", >making it look more like a cifs.ko struct by using the standard "cifs_" >prefix. > >Also upgrades from Camel_Case to snake_case. > >Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de> >--- >fs/cifs/asn1.c | 4 +- >fs/cifs/cifs_debug.c | 14 ++-- >fs/cifs/cifs_debug.h | 4 +- >fs/cifs/cifs_spnego.c | 2 +- >fs/cifs/cifs_spnego.h | 2 +- >fs/cifs/cifs_swn.h | 8 +-- >fs/cifs/cifsencrypt.c | 14 ++-- >fs/cifs/cifsfs.c | 6 +- >fs/cifs/cifsglob.h | 140 ++++++++++++++++++++-------------------- >fs/cifs/cifsproto.h | 114 ++++++++++++++++---------------- >fs/cifs/cifssmb.c | 24 +++---- >fs/cifs/connect.c | 134 +++++++++++++++++++------------------- >fs/cifs/dfs_cache.c | 10 +-- >fs/cifs/dir.c | 6 +- >fs/cifs/file.c | 60 ++++++++--------- >fs/cifs/fscache.c | 2 +- >fs/cifs/inode.c | 22 +++---- >fs/cifs/ioctl.c | 2 +- >fs/cifs/link.c | 4 +- >fs/cifs/misc.c | 10 +-- >fs/cifs/netmisc.c | 2 +- >fs/cifs/ntlmssp.h | 6 +- >fs/cifs/readdir.c | 4 +- >fs/cifs/sess.c | 52 +++++++-------- >fs/cifs/smb1ops.c | 26 ++++---- >fs/cifs/smb2inode.c | 2 +- >fs/cifs/smb2misc.c | 14 ++-- >fs/cifs/smb2ops.c | 86 ++++++++++++------------ >fs/cifs/smb2pdu.c | 122 +++++++++++++++++----------------- >fs/cifs/smb2proto.h | 58 ++++++++--------- >fs/cifs/smb2transport.c | 44 ++++++------- >fs/cifs/smbdirect.c | 10 +-- >fs/cifs/smbdirect.h | 16 ++--- >fs/cifs/transport.c | 48 +++++++------- >34 files changed, 536 insertions(+), 536 deletions(-)
On 7/25/2022 10:41 PM, Enzo Matsumiya wrote: > On 07/25, Tom Talpey wrote: >> On 7/25/2022 6:36 PM, Enzo Matsumiya wrote: >>> Hi all, >>> >>> This patch set (v2) renames several cifs.ko data structures, >>> variables, and >>> functions with the goal to improve readability of the code. >>> >>> In summary, what's been done: >>> - change from CamelCase to snake_case >>> - try to give more meaning to globals and struct members >>> - rename status fields (server, ses, tcon), define constants for the >>> various statuses (4/11 can be shared between those structs, others are >>> specific to each) >>> - rename of list_head variables to better represent whether they'are >>> used as a list element ("head") or a list per se. Also tried to give >>> more meaning to these, as "rlist", "tlist", "llist" looked confusing >>> and, sometimes, ambiguous. >>> - remove redundant prefixes from struct members name, e.g. >>> tcon_tlink's tl_*, smb_rqst's rq_*, cifs_fattr's cf_*, etc >>> >>> No functional changes has been made. >>> >>> I know these touch some very old code that older devs are highly used >>> to, but I see this as an improvement to reading the code for everyone. >>> >>> I'll be waiting for your reviews and feedback. >> >> Enzo, I think this effort is great. If you combine this modernization >> with refactoring SMB1 into separate files to make it easier to do >> away with entirely, I'll be even more supportive. > > Tom, thanks for your feedback! > As a matter of fact, I do have a branch with SMB1 code isolated [*] and the > module renamed to "SMBFS" to modernize this module, while also keeping > it SMB-version-agnostic. > > I'm sending my changes gradually so I don't have to change a lot of > things in case there's a lot of negative feedback. > > > [*] - current discussions: > - is "smbfs" the best module name? Steve suggested "smb3", but, again, > this ties the module to a specific SMB version I think "smbfs" is more consistent with other kmods. And I'd be careful about a specific version like "smb3", because that paints it into the corner if it also supports smb2, or future dialects. > - should SMB1 code be isolated as in source-code only or should it be a > different object (i.e. only built through kernel config and > "disable_legacy_dialects" wiped away)? Or even a separate module? There's no single answer here, but my personal view is that it should become a separate module. As a compile-time option, the distros are basically forced to turn it on. We don't want that. As a module, they can choose to leave it out. We do want that. > In any case, I'm keeping "cifs" as a module alias for SMB1 code for now, > and I think we'll need it that way for some time, but at least the > internal migration will be done by then. Yes, it's definitely needed (for now) to maintain a "cifs" module alias, because user scripts might be using it to modload. In future if smb1 becomes a module, then drop "cifs" entirely. Tom. > Thanks again, > > Enzo > >>> v2: >>> - remove status typedefs (suggested by Christoph Hellwig) >>> - define status constants instead, reuse some between different >>> structs so we don't have to create a different set of statuses >>> for each cifs struct >>> >>> Enzo Matsumiya (10): >>> cifs: rename xid/mid globals >>> cifs: rename global counters >>> cifs: rename "TCP_Server_Info" struct to "cifs_server_info" >>> cifs: rename cifs{File,Lock,Inode}Info structs and more >>> cifs: convert server info vars to snake_case >>> cifs: change status and security types enums to constants >>> cifs: rename cifsFYI to debug_level >>> cifs: rename list_head fields >>> cifs: rename more CamelCase to snake_case >>> cifs: rename more list_heads, remove redundant prefixes >>> >>> fs/cifs/Kconfig | 2 +- >>> fs/cifs/asn1.c | 4 +- >>> fs/cifs/cifs_debug.c | 158 ++++----- >>> fs/cifs/cifs_debug.h | 29 +- >>> fs/cifs/cifs_spnego.c | 4 +- >>> fs/cifs/cifs_spnego.h | 2 +- >>> fs/cifs/cifs_swn.c | 24 +- >>> fs/cifs/cifs_swn.h | 8 +- >>> fs/cifs/cifs_unicode.c | 4 +- >>> fs/cifs/cifs_unicode.h | 2 +- >>> fs/cifs/cifsacl.c | 22 +- >>> fs/cifs/cifsencrypt.c | 78 ++--- >>> fs/cifs/cifsfs.c | 124 +++---- >>> fs/cifs/cifsglob.h | 694 ++++++++++++++++++++-------------------- >>> fs/cifs/cifsproto.h | 172 +++++----- >>> fs/cifs/cifssmb.c | 356 ++++++++++----------- >>> fs/cifs/connect.c | 574 ++++++++++++++++----------------- >>> fs/cifs/dfs_cache.c | 178 +++++------ >>> fs/cifs/dfs_cache.h | 40 +-- >>> fs/cifs/dir.c | 16 +- >>> fs/cifs/file.c | 636 ++++++++++++++++++------------------ >>> fs/cifs/fs_context.c | 8 +- >>> fs/cifs/fs_context.h | 2 +- >>> fs/cifs/fscache.c | 18 +- >>> fs/cifs/fscache.h | 10 +- >>> fs/cifs/inode.c | 530 +++++++++++++++--------------- >>> fs/cifs/ioctl.c | 18 +- >>> fs/cifs/link.c | 26 +- >>> fs/cifs/misc.c | 185 ++++++----- >>> fs/cifs/netmisc.c | 4 +- >>> fs/cifs/ntlmssp.h | 6 +- >>> fs/cifs/readdir.c | 344 ++++++++++---------- >>> fs/cifs/sess.c | 142 ++++---- >>> fs/cifs/smb1ops.c | 182 +++++------ >>> fs/cifs/smb2file.c | 36 +-- >>> fs/cifs/smb2inode.c | 136 ++++---- >>> fs/cifs/smb2maperror.c | 2 +- >>> fs/cifs/smb2misc.c | 72 ++--- >>> fs/cifs/smb2ops.c | 555 ++++++++++++++++---------------- >>> fs/cifs/smb2pdu.c | 596 +++++++++++++++++----------------- >>> fs/cifs/smb2proto.h | 68 ++-- >>> fs/cifs/smb2transport.c | 112 +++---- >>> fs/cifs/smbdirect.c | 28 +- >>> fs/cifs/smbdirect.h | 16 +- >>> fs/cifs/transport.c | 236 +++++++------- >>> fs/cifs/xattr.c | 12 +- >>> 46 files changed, 3230 insertions(+), 3241 deletions(-) >>> >
Hi all, This patch set (v2) renames several cifs.ko data structures, variables, and functions with the goal to improve readability of the code. In summary, what's been done: - change from CamelCase to snake_case - try to give more meaning to globals and struct members - rename status fields (server, ses, tcon), define constants for the various statuses (4/11 can be shared between those structs, others are specific to each) - rename of list_head variables to better represent whether they'are used as a list element ("head") or a list per se. Also tried to give more meaning to these, as "rlist", "tlist", "llist" looked confusing and, sometimes, ambiguous. - remove redundant prefixes from struct members name, e.g. tcon_tlink's tl_*, smb_rqst's rq_*, cifs_fattr's cf_*, etc No functional changes has been made. I know these touch some very old code that older devs are highly used to, but I see this as an improvement to reading the code for everyone. I'll be waiting for your reviews and feedback. Cheers, Enzo v2: - remove status typedefs (suggested by Christoph Hellwig) - define status constants instead, reuse some between different structs so we don't have to create a different set of statuses for each cifs struct Enzo Matsumiya (10): cifs: rename xid/mid globals cifs: rename global counters cifs: rename "TCP_Server_Info" struct to "cifs_server_info" cifs: rename cifs{File,Lock,Inode}Info structs and more cifs: convert server info vars to snake_case cifs: change status and security types enums to constants cifs: rename cifsFYI to debug_level cifs: rename list_head fields cifs: rename more CamelCase to snake_case cifs: rename more list_heads, remove redundant prefixes fs/cifs/Kconfig | 2 +- fs/cifs/asn1.c | 4 +- fs/cifs/cifs_debug.c | 158 ++++----- fs/cifs/cifs_debug.h | 29 +- fs/cifs/cifs_spnego.c | 4 +- fs/cifs/cifs_spnego.h | 2 +- fs/cifs/cifs_swn.c | 24 +- fs/cifs/cifs_swn.h | 8 +- fs/cifs/cifs_unicode.c | 4 +- fs/cifs/cifs_unicode.h | 2 +- fs/cifs/cifsacl.c | 22 +- fs/cifs/cifsencrypt.c | 78 ++--- fs/cifs/cifsfs.c | 124 +++---- fs/cifs/cifsglob.h | 694 ++++++++++++++++++++-------------------- fs/cifs/cifsproto.h | 172 +++++----- fs/cifs/cifssmb.c | 356 ++++++++++----------- fs/cifs/connect.c | 574 ++++++++++++++++----------------- fs/cifs/dfs_cache.c | 178 +++++------ fs/cifs/dfs_cache.h | 40 +-- fs/cifs/dir.c | 16 +- fs/cifs/file.c | 636 ++++++++++++++++++------------------ fs/cifs/fs_context.c | 8 +- fs/cifs/fs_context.h | 2 +- fs/cifs/fscache.c | 18 +- fs/cifs/fscache.h | 10 +- fs/cifs/inode.c | 530 +++++++++++++++--------------- fs/cifs/ioctl.c | 18 +- fs/cifs/link.c | 26 +- fs/cifs/misc.c | 185 ++++++----- fs/cifs/netmisc.c | 4 +- fs/cifs/ntlmssp.h | 6 +- fs/cifs/readdir.c | 344 ++++++++++---------- fs/cifs/sess.c | 142 ++++---- fs/cifs/smb1ops.c | 182 +++++------ fs/cifs/smb2file.c | 36 +-- fs/cifs/smb2inode.c | 136 ++++---- fs/cifs/smb2maperror.c | 2 +- fs/cifs/smb2misc.c | 72 ++--- fs/cifs/smb2ops.c | 555 ++++++++++++++++---------------- fs/cifs/smb2pdu.c | 596 +++++++++++++++++----------------- fs/cifs/smb2proto.h | 68 ++-- fs/cifs/smb2transport.c | 112 +++---- fs/cifs/smbdirect.c | 28 +- fs/cifs/smbdirect.h | 16 +- fs/cifs/transport.c | 236 +++++++------- fs/cifs/xattr.c | 12 +- 46 files changed, 3230 insertions(+), 3241 deletions(-)