[PATCH 2/2] vfs_glusterfs: Samba VFS module for glusterfs
Anand Avati
avati at redhat.com
Thu Apr 25 17:22:27 MDT 2013
On 04/25/2013 02:32 AM, Volker Lendecke wrote:
>> +glfd_fd_store (glfs_fd_t *glfd)
>> +{
>> + int i;
>> + void *tmp;
>> +
>> + if (glfd_fd_size == glfd_fd_used) {
>> + tmp = realloc (glfd_fd, glfd_fd_size + 1024);
>
> Is this correct? Shouldn't that be
>
> tmp = realloc (glfd_fd, (glfd_fd_size + 1024) * sizeof(glfs_fd_t *));
Doh! you're right. This wouldn't even show up as a problem until you
have more than 128 (or 256 on 32bit) concurrently open fds. Thanks for
catching :-)
>> +static glfs_fd_t *
>> +glfd_fd_get (int i)
>> +{
>> + return glfd_fd[i];
>
> I'd feel better with a size check here.
Ack.
>> +}
>> +
>> +static glfs_fd_t *
>> +glfd_fd_clear (int i)
>> +{
>> + glfs_fd_t *glfd = glfd_fd[i];
>
> Same here for the size check.
Ack.
>> +
>> + glfd_fd[i] = 0;
>> + glfd_fd_used--;
>> + return glfd;
>> +}
>> +
>> +
>> +/* Helper to convert stat to stat_ex */
>> +
>> +static void
>> +smb_stat_ex_from_stat (struct stat_ex *dst, const struct stat *src)
>> +{
>> + memset (dst, 0, sizeof (*dst));
>
> More Samba-like would be ZERO_STRUCTP(dst).
OK.
>> +static struct dirent *
>> +vfs_gluster_readdir (struct vfs_handle_struct *handle, DIR *dirp,
>> + SMB_STRUCT_STAT *sbuf)
>> +{
>> + static char direntbuf[512];
>> + int ret;
>> + struct stat stat;
>> + struct dirent *dirent = 0;
>> +
>> + ret = glfs_readdirplus_r ((void *)dirp,&stat, (void *)direntbuf,
>> + &dirent);
>> + if (ret)
>> + dirent = NULL;
>> +
>> + if (sbuf)
>> + smb_stat_ex_from_stat (sbuf,&stat);
>
> Do you initialize the stat buf even in case of an error?
We don't. This would result in garbage getting written into sbuf
"safely" (without crashing, without expectation of sbuf having any
expected values). But I will fix it anyways.
Thanks for the review!
Avati
More information about the samba-technical
mailing list