Re: [PATCH v2 2/4 incubator] smb: Remove _FILE_OFFSET_BITS and be compatible with samba

Damien Zammit, le dim. 09 août 2026 07:50:12 +0000, a ecrit:
> diff --git a/smbfs.c b/smbfs.c
> index 161c57c17..373324466 100644
> --- a/smbfs.c
> +++ b/smbfs.c
> @@ -32,7 +32,7 @@ extern void smbfs_terminate ();
>  static struct argp_option options[] = 
>  {
>  	{"server",'s',"SERVER",0, "SMB server"},
> -	{"resource",'r',"RESOURCE",0, "directory to access"},
> +	{"resource",'r',"RESOURCE",0, "share to access"},

This looks unrelated?

>  	{"password",'p',"PWD",0, "password to use (default: empty password)"},
>  	{"username",'u',"USR",0, "user name to use (default: `$USER')"},
>  	{"workgroup",'w',"WKG",0, "workgroup to use (default: `WORKGROUP')"},
> @@ -40,7 +40,9 @@ static struct argp_option options[] =
>  	{0}
>  };
>  static error_t parse_opt(int key, char *arg, struct argp_state *state)
> -{	
> +{
> +  char share[1024];
> +
>    switch(key)
>      {
>        case 's':
> @@ -64,6 +66,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
>        case ARGP_KEY_ARG:
>          break;
>        case ARGP_KEY_END:
> +        snprintf(share, 1024, "smb://%s/%s", opts.server, opts.share);

Better use asprintf to avoid hardcoding a length.

Also, this is unrelated.

> +        opts.share = strdup(share);
>          break;
>        default:
>          return ARGP_ERR_UNKNOWN;
> @@ -101,9 +105,13 @@ main (int argc, char *argv[])
>    netfs_startup(bootstrap, 0);
>    smbfs_init();
>  
> +  fprintf(stderr, "Starting a smbfs\n");
> +
>    for(;;)
>      netfs_server_loop ();
>  
>    smbfs_terminate ();
> +
> +  fprintf(stderr, "Stopped a smbfs\n");
>    return 0;
>  }

This looks unrelated.

> diff --git a/smbnetfs.c b/smbnetfs.c
> index 70bebc73e..dc8269dd8 100644
> --- a/smbnetfs.c
> +++ b/smbnetfs.c
> @@ -63,14 +63,51 @@ struct netnode
>    struct node *entries;				/* entries, if a directory */
>  };
>  
> -/* Return a zeroed stat buffer for CRED.  */
> +/* Downsize a stat64 structure to a stat for samba compat */
>  static struct stat
> -empty_stat (void)
> +stat64_to_stat (struct stat64 *st64)
>  {
>    struct stat st;
>  
>    memset (&st, 0, sizeof st);
>  
> +  if (sizeof(off_t) == 4)
> +    {
> +      if ((st64->st_ino > 0xffffffff)
> +       || (st64->st_size > 0x7fffffff)
> +       || (st64->st_blocks > 0x7fffffff))
> +        return st;

Rather make the function take the struct stat to be filled as parameter,
and return an error code.

> +    }
> +
> +  st.st_fstype = st64->st_fstype;
> +  st.st_fsid = st64->st_fsid;
> +  st.st_ino = st64->st_ino;
> +  st.st_gen = st64->st_gen;
> +  st.st_rdev = st64->st_rdev;
> +  st.st_mode = st64->st_mode;
> +  st.st_nlink = st64->st_nlink;
> +  st.st_uid = st64->st_uid;
> +  st.st_gid = st64->st_gid;
> +  st.st_size = st64->st_size;
> +  st.st_atime = st64->st_atime;
> +  st.st_mtime = st64->st_mtime;
> +  st.st_ctime = st64->st_ctime;
> +  st.st_blksize = st64->st_blksize;
> +  st.st_blocks = st64->st_blocks;
> +  st.st_author = st64->st_author;
> +  st.st_flags = st64->st_flags;
> +
> +  return st;
> +}


> @@ -605,10 +698,10 @@ netfs_attempt_read (struct iouser * cred, struct node * np, loff_t offset,
>      }
>  
>    pthread_mutex_lock (&smb_mutex);
> -  ret = smbc_getFunctionLseek(ctx) (ctx, fd, offset, SEEK_SET);
> +  ret = smbc_getFunctionLseek(ctx) (ctx, fd, offset2, SEEK_SET);
>    pthread_mutex_unlock (&smb_mutex);
>    
> -  if ((ret < 0) || (ret != offset))
> +  if ((ret < 0) || (ret != offset2))

That's unrelated, but now realizing: is smbfs serving with
multiple threads? If so, concurrent netfs_attempt_read would lseek
concurrently... Does samba still not provide a pread-like interface?

> @@ -874,8 +980,13 @@ netfs_get_dirents (struct iouser *cred, struct node *dir, int entry,
>            }
>          else if (!strcmp (dirent->name, ".."))
>            {
> -	    st = empty_stat ();
> -	    st.st_mode |= S_IFDIR;
> +	    st64 = empty_stat ();
> +	    st64.st_mode |= S_IFDIR;
> +
> +            /* Reduce stat64 to stat, or fail if values too big */
> +            st = stat64_to_stat (&st64);
> +            if (!st.st_fstype)
> +              err = errno = E2BIG;
>            }
>          else
>            {
> @@ -893,8 +1004,13 @@ netfs_get_dirents (struct iouser *cred, struct node *dir, int entry,
>  	    if (err)
>  	      {
>  		/* STAT_FILE_NAME is not accessible but ought to be listed.  */
> -		st = empty_stat ();
> +		st64 = empty_stat ();
>  		err = 0;
> +
> +                /* Reduce stat64 to stat, or fail if values too big */
> +                st = stat64_to_stat (&st64);
> +                if (!st.st_fstype)
> +                  err = errno = E2BIG;
>  	      }
>            }

I don't see why reducing here? We are starting with an empty stat64
already. A reduced stat will be just an empty stat.
(modulo the S_IFDIR flag which won't pose problem)

Samuel