Re: [WIP PATCH][CIFS] move legacy cifs (smb1) code to legacy ifdef and do not include in build when legacy disabled

[Steve French <smfrench@gmail.com>]

On Mon, Aug 1, 2022 at 7:15 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> On 08/01, ronnie sahlberg wrote:
> >Small nit : in cifssmb.c  why comment out smb2proto.h,  just delete the line.
>
> Agreed.

Makes sense. Will fix this.


> @Steve also, why ifdef everything instead of putting everything in a
> "smb1" dir and just use Kconfig to build that dir? IOW like I did in in
> my branch https://github.com/ematsumiya/linux/commits/refactor

moving things to smb1 dir seems to be more related to the eventual
move to having two
modules instead of one, e.g. cifs.ko and an smb3.ko (or smbfs.ko or
whatever we call it,
today mounts can do "mount -t smb3" or "mount -t cifs" but we don't
have separate modules for those).
The other reason is that we don't have enough cifs (smb1 only) files
for that to make sense yet.
There are only two smb1 only C files. This patch adds the second one
cifssmb.c (last release I added the
first one smb1ops.c):

cifs-$(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) += smb1ops.o cifssmb.o

We can probably split some headers out, but this step is needed first
(isolating more SMB1/CIFS
code from SMB3 code, and moving it where possible into SMB1/CIFS
specific c files).

> why ifdef everything

Most of the code is in cifssmb.c and smb1ops.c but there are 58 "ifdef
CONFIG_CIFS_ALLOW_INSECURE_LEGACY"
but fewer than 20 of these are distinct functions (those we can move
to cifssmb.c or a new smb1 helper c file) so most
of those require code cleanup in those functions - and most are due to
functions that have hard to read patterns like:

if (smb1 unix extensions) {
           do some smb1 specific stuff which should be in a helper function
           return;
}

call some dialect specific helper function

or similarly smb1 code wedged in dialect neutral code in
file.c/dir.c/inode.c that we need to move e.g.

                if (backup_cred(cifs_sb) && is_smb1_server(server)) {
                        FILE_DIRECTORY_INFO *fdi;
                        SEARCH_ID_FULL_DIR_INFO *si;

                        rc = cifs_backup_query_path_info(xid, tcon, sb,
                                                         full_path,
                                                         &smb1_backup_rsp_buf,
                                                         &data);

Let's do follow on patches to cleanup code like this in file.c/dir.c/inode.c
-- 
Thanks,

Steve