I have thought about this a few times, but always hesitated because I
was hoping that eventually cifs mount option parsing could use new
parsing functions (which would also shrink the "parse_mount_options"
routine which we call during cifs mount), and would cause us to
rewrite this. IIRC nfs was changed a few years ago to use the new
mount mechanism (match_table and tokens).
On Sun, Nov 30, 2008 at 12:40 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> struct smb_vol is fairly large, it's probably best to kzalloc it...
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/cifs/connect.c | 91 ++++++++++++++++++++++++++++------------------------
> 1 files changed, 49 insertions(+), 42 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 291ccd6..b08c6a3 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2186,27 +2186,30 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
> {
> int rc = 0;
> int xid;
> - struct smb_vol volume_info;
> + struct smb_vol *volume_info;
> struct cifsSesInfo *pSesInfo = NULL;
> struct cifsTconInfo *tcon = NULL;
> struct TCP_Server_Info *srvTcp = NULL;
>
> xid = GetXid();
>
> -/* cFYI(1, ("Entering cifs_mount. Xid: %d with: %s", xid, mount_data)); */
> + volume_info = kzalloc(sizeof(struct smb_vol), GFP_KERNEL);
> + if (!volume_info) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> - memset(&volume_info, 0, sizeof(struct smb_vol));
> - if (cifs_parse_mount_options(mount_data, devname, &volume_info)) {
> + if (cifs_parse_mount_options(mount_data, devname, volume_info)) {
> rc = -EINVAL;
> goto out;
> }
>
> - if (volume_info.nullauth) {
> + if (volume_info->nullauth) {
> cFYI(1, ("null user"));
> - volume_info.username = "";
> - } else if (volume_info.username) {
> + volume_info->username = "";
> + } else if (volume_info->username) {
> /* BB fixme parse for domain name here */
> - cFYI(1, ("Username: %s", volume_info.username));
> + cFYI(1, ("Username: %s", volume_info->username));
> } else {
> cifserror("No username specified");
> /* In userspace mount helper we can get user name from alternate
> @@ -2217,27 +2220,27 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
>
>
> /* this is needed for ASCII cp to Unicode converts */
> - if (volume_info.iocharset == NULL) {
> + if (volume_info->iocharset == NULL) {
> cifs_sb->local_nls = load_nls_default();
> /* load_nls_default can not return null */
> } else {
> - cifs_sb->local_nls = load_nls(volume_info.iocharset);
> + cifs_sb->local_nls = load_nls(volume_info->iocharset);
> if (cifs_sb->local_nls == NULL) {
> cERROR(1, ("CIFS mount error: iocharset %s not found",
> - volume_info.iocharset));
> + volume_info->iocharset));
> rc = -ELIBACC;
> goto out;
> }
> }
>
> /* get a reference to a tcp session */
> - srvTcp = cifs_get_tcp_session(&volume_info);
> + srvTcp = cifs_get_tcp_session(volume_info);
> if (IS_ERR(srvTcp)) {
> rc = PTR_ERR(srvTcp);
> goto out;
> }
>
> - pSesInfo = cifs_find_smb_ses(srvTcp, volume_info.username);
> + pSesInfo = cifs_find_smb_ses(srvTcp, volume_info->username);
> if (pSesInfo) {
> cFYI(1, ("Existing smb sess found (status=%d)",
> pSesInfo->status));
> @@ -2275,24 +2278,24 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
> list_add(&pSesInfo->smb_ses_list, &srvTcp->smb_ses_list);
> write_unlock(&cifs_tcp_ses_lock);
>
> - /* volume_info.password freed at unmount */
> - if (volume_info.password) {
> - pSesInfo->password = volume_info.password;
> + /* volume_info->password freed at unmount */
> + if (volume_info->password) {
> + pSesInfo->password = volume_info->password;
> /* set to NULL to prevent freeing on exit */
> - volume_info.password = NULL;
> + volume_info->password = NULL;
> }
> - if (volume_info.username)
> - strncpy(pSesInfo->userName, volume_info.username,
> + if (volume_info->username)
> + strncpy(pSesInfo->userName, volume_info->username,
> MAX_USERNAME_SIZE);
> - if (volume_info.domainname) {
> - int len = strlen(volume_info.domainname);
> + if (volume_info->domainname) {
> + int len = strlen(volume_info->domainname);
> pSesInfo->domainName = kmalloc(len + 1, GFP_KERNEL);
> if (pSesInfo->domainName)
> strcpy(pSesInfo->domainName,
> - volume_info.domainname);
> + volume_info->domainname);
> }
> - pSesInfo->linux_uid = volume_info.linux_uid;
> - pSesInfo->overrideSecFlg = volume_info.secFlg;
> + pSesInfo->linux_uid = volume_info->linux_uid;
> + pSesInfo->overrideSecFlg = volume_info->secFlg;
> down(&pSesInfo->sesSem);
>
> /* BB FIXME need to pass vol->secFlgs BB */
> @@ -2303,14 +2306,14 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
>
> /* search for existing tcon to this server share */
> if (!rc) {
> - setup_cifs_sb(&volume_info, cifs_sb);
> + setup_cifs_sb(volume_info, cifs_sb);
>
> - tcon = cifs_find_tcon(pSesInfo, volume_info.UNC);
> + tcon = cifs_find_tcon(pSesInfo, volume_info->UNC);
> if (tcon) {
> cFYI(1, ("Found match on UNC path"));
> /* existing tcon already has a reference */
> cifs_put_smb_ses(pSesInfo);
> - if (tcon->seal != volume_info.seal)
> + if (tcon->seal != volume_info->seal)
> cERROR(1, ("transport encryption setting "
> "conflicts with existing tid"));
> } else {
> @@ -2322,8 +2325,8 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
> tcon->ses = pSesInfo;
>
> /* check for null share name ie connect to dfs root */
> - if ((strchr(volume_info.UNC + 3, '\\') == NULL)
> - && (strchr(volume_info.UNC + 3, '/') == NULL)) {
> + if ((strchr(volume_info->UNC + 3, '\\') == NULL)
> + && (strchr(volume_info->UNC + 3, '/') == NULL)) {
> /* rc = connect_to_dfs_path(...) */
> cFYI(1, ("DFS root not supported"));
> rc = -ENODEV;
> @@ -2332,10 +2335,10 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
> /* BB Do we need to wrap sesSem around
> * this TCon call and Unix SetFS as
> * we do on SessSetup and reconnect? */
> - rc = CIFSTCon(xid, pSesInfo, volume_info.UNC,
> + rc = CIFSTCon(xid, pSesInfo, volume_info->UNC,
> tcon, cifs_sb->local_nls);
> cFYI(1, ("CIFS Tcon rc = %d", rc));
> - if (volume_info.nodfs) {
> + if (volume_info->nodfs) {
> tcon->Flags &= ~SMB_SHARE_IS_IN_DFS;
> cFYI(1, ("DFS disabled (%d)",
> tcon->Flags));
> @@ -2343,7 +2346,7 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
> }
> if (rc)
> goto mount_fail_check;
> - tcon->seal = volume_info.seal;
> + tcon->seal = volume_info->seal;
> write_lock(&cifs_tcp_ses_lock);
> list_add(&tcon->tcon_list, &pSesInfo->tcon_list);
> write_unlock(&cifs_tcp_ses_lock);
> @@ -2353,9 +2356,9 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
> to a share so for resources mounted more than once
> to the same server share the last value passed in
> for the retry flag is used */
> - tcon->retry = volume_info.retry;
> - tcon->nocase = volume_info.nocase;
> - tcon->local_lease = volume_info.local_lease;
> + tcon->retry = volume_info->retry;
> + tcon->nocase = volume_info->nocase;
> + tcon->local_lease = volume_info->local_lease;
> }
> if (pSesInfo) {
> if (pSesInfo->capabilities & CAP_LARGE_FILES) {
> @@ -2392,7 +2395,7 @@ mount_fail_check:
> if (tcon->ses->capabilities & CAP_UNIX)
> /* reset of caps checks mount to see if unix extensions
> disabled for just this mount */
> - reset_cifs_unix_caps(xid, tcon, sb, &volume_info);
> + reset_cifs_unix_caps(xid, tcon, sb, volume_info);
> else
> tcon->unix_ext = 0; /* server does not support them */
>
> @@ -2411,18 +2414,22 @@ mount_fail_check:
> cifs_sb->rsize = min(cifs_sb->rsize,
> (tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE));
>
> - /* volume_info.password is freed above when existing session found
> + /* volume_info->password is freed above when existing session found
> (in which case it is not needed anymore) but when new sesion is created
> the password ptr is put in the new session structure (in which case the
> password will be freed at unmount time) */
> out:
> /* zero out password before freeing */
> - if (volume_info.password != NULL) {
> - memset(volume_info.password, 0, strlen(volume_info.password));
> - kfree(volume_info.password);
> + if (volume_info) {
> + if (volume_info->password != NULL) {
> + memset(volume_info->password, 0,
> + strlen(volume_info->password));
> + kfree(volume_info->password);
> + }
> + kfree(volume_info->UNC);
> + kfree(volume_info->prepath);
> + kfree(volume_info);
> }
> - kfree(volume_info.UNC);
> - kfree(volume_info.prepath);
> FreeXid(xid);
> return rc;
> }
> --
> 1.5.5.1
>
>
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
