Re: [patch 2/2] cifs - cifs_find_tcp_session cleanup

[Cyrill Gorcunov]

On Wed, May 7, 2008 at 2:07 AM, Steve French <smfrench@xxxxxxxxx> wrote:
> This looks like it would fail in the ipv6 case - because
>  target_ip_addr (the ipv4 address) would be null and it would exit
>  before checking.
>
>
>
>
>
>  On Thu, May 1, 2008 at 10:48 AM, Cyrill Gorcunov <gorcunov@xxxxxxxxx> wrote:
>  > This patch is to remove too indented code in cifs_find_tcp_session.
>  >  Also memcmp was used wrongly - we would have found the session
>  >  which IP6 addresses didn't match.
>  >
>  >  Signed-off-by: Cyrill Gorcunov <gorcunov@xxxxxxxxx>
>  >  ---
>  >
>  >  Index: linux-2.6.git/fs/cifs/connect.c
>  >  ===================================================================
>  >  --- linux-2.6.git.orig/fs/cifs/connect.c        2008-05-01 19:02:51.000000000 +0400
>  >  +++ linux-2.6.git/fs/cifs/connect.c     2008-05-01 19:36:54.000000000 +0400
>  >  @@ -1318,42 +1318,44 @@ cifs_parse_mount_options(char *options,
>  >
>  >   static struct cifsSesInfo *
>  >   cifs_find_tcp_session(struct in_addr *target_ip_addr,
>  >  -               struct in6_addr *target_ip6_addr,
>  >  -                char *userName, struct TCP_Server_Info **psrvTcp)
>  >  +                     struct in6_addr *target_ip6_addr,
>  >  +                     char *userName, struct TCP_Server_Info **psrvTcp)
>  >   {
>  >         struct list_head *tmp;
>  >         struct cifsSesInfo *ses;
>  >  +
>  >         *psrvTcp = NULL;
>  >  -       read_lock(&GlobalSMBSeslock);
>  >
>  >  +       if (!target_ip_addr)
>  >  +               return NULL;
>  >  +
>  >  +       read_lock(&GlobalSMBSeslock);
>  >         list_for_each(tmp, &GlobalSMBSessionList) {
>  >                 ses = list_entry(tmp, struct cifsSesInfo, cifsSessionList);
>  >  -               if (ses->server) {
>  >  -                       if ((target_ip_addr &&
>  >  -                               (ses->server->addr.sockAddr.sin_addr.s_addr
>  >  -                                 == target_ip_addr->s_addr)) || (target_ip6_addr
>  >  -                               && memcmp(&ses->server->addr.sockAddr6.sin6_addr,
>  >  -                                       target_ip6_addr, sizeof(*target_ip6_addr)))) {
>  >  -                               /* BB lock server and tcp session and increment
>  >  -                                     use count here?? */
>  >  -
>  >  -                               /* found a match on the TCP session */
>  >  -                               *psrvTcp = ses->server;
>  >  -
>  >  -                               /* BB check if reconnection needed */
>  >  -                               if (strncmp
>  >  -                                   (ses->userName, userName,
>  >  -                                    MAX_USERNAME_SIZE) == 0){
>  >  -                                       read_unlock(&GlobalSMBSeslock);
>  >  -                                       /* Found exact match on both TCP and
>  >  -                                          SMB sessions */
>  >  -                                       return ses;
>  >  -                               }
>  >  -                       }
>  >  -               }
>  >  -               /* else tcp and smb sessions need reconnection */
>  >  +               if (!ses->server)
>  >  +                       continue;
>  >  +
>  >  +               if (ses->server->addr.sockAddr.sin_addr.s_addr != target_ip_addr->s_addr ||
>  >  +                   memcmp(&ses->server->addr.sockAddr6.sin6_addr,
>  >  +                          target_ip6_addr, sizeof(*target_ip6_addr)))
>  >  +                       continue;
>  >  +
>  >  +               /* BB lock server and tcp session and increment
>  >  +                  use count here?? */
>  >  +
>  >  +               /* found a match on the TCP session */
>  >  +               *psrvTcp = ses->server;
>  >  +
>  >  +               /* BB check if reconnection needed */
>  >  +               if (strncmp(ses->userName, userName, MAX_USERNAME_SIZE))
>  >  +                       continue;
>  >  +
>  >  +               /* Found exact match on both TCP and SMB sessions */
>  >  +               read_unlock(&GlobalSMBSeslock);
>  >  +               return ses;
>  >         }
>  >         read_unlock(&GlobalSMBSeslock);
>  >  +
>  >         return NULL;
>  >   }
>  >
>  >
>  >  --
>  >
>
>
>
>  --
>  Thanks,
>
>  Steve
>

Ah, indeed! Thanks, Steve, I'm really sorry for this.
Will remake it.

Andrew, could you drop it from -mm please?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/