Merge pull request #513 from fizmat/extra-checks

XRootDFS: Extra checks and error reporting

src/XrdFfs/XrdFfsMisc.cc

@@ -119,7 +119,7 @@ int XrdFfsMisc_get_all_urls_real(const char *oldurl, char **newurls, const int n
    used a cache to reduce unnecessary queries to the redirector 
 */ 
 
-char XrdFfsMiscCururl[1024] = "";
+char XrdFfsMiscCururl[MAXROOTURLLEN] = "";
 char *XrdFfsMiscUrlcache[XrdFfs_MAX_NUM_NODES];
 int XrdFfsMiscNcachedurls = 0;
 time_t XrdFfsMiscUrlcachetime = 0;
@@ -211,7 +211,7 @@ int XrdFfsMisc_get_list_of_data_servers(char* list)
     const char *hName, *hNend, *hPort, *hPend;
     char *url, *rc, *hostip, hsep;
 
-    rc = (char*)malloc(sizeof(char) * XrdFfs_MAX_NUM_NODES * 1024);
+    rc = (char*)malloc(sizeof(char) * XrdFfs_MAX_NUM_NODES * MAXROOTURLLEN);
     rc[0] = '\0';
     pthread_mutex_lock(&XrdFfsMiscUrlcache_mutex);
     for (i = 0; i < XrdFfsMiscNcachedurls; i++)
@@ -418,7 +418,7 @@ void XrdFfsMisc_xrd_secsss_register(uid_t user_uid, gid_t user_gid, int *id)
 
 void XrdFfsMisc_xrd_secsss_editurl(char *url, uid_t user_uid, int *id)
 {
-    char user_num[9], nurl[1024], *tmp;
+    char user_num[9], nurl[MAXROOTURLLEN], *tmp;
 
 // Xrootd proxy also use some of the stat()/unlink(), etc funcitons here, without user "sss". It use the default 
 // connection login name. Don't change this behavior. (so for proxy, use no "sss", and always have *id = 0
@@ -431,11 +431,11 @@ void XrdFfsMisc_xrd_secsss_editurl(char *url, uid_t user_uid, int *id)
         else user_num[strlen(user_num)] = *id + 48;
 
         nurl[0] = '\0';
-        strcat(nurl, "root://");
-        strcat(nurl, user_num);
-        strcat(nurl, "@");
-        strcat(nurl, &(url[7])); 
-        strcpy(url, nurl);
+        strncat(nurl, "root://", 7);
+        strncat(nurl, user_num, 9);
+        strncat(nurl, "@", 1);
+        strncat(nurl, &(url[7]), MAXROOTURLLEN-17);
+        strncpy(url, nurl, MAXROOTURLLEN);
     }
 }
 

src/XrdFfs/XrdFfsWcache.cc

@@ -102,7 +102,14 @@ void XrdFfsWcache_init(int basefd, int maxfd)
     }
 }
 
-int XrdFfsWcache_create(int fd) 
+int XrdFfsWcache_create(int fd)
+/* Create a write cache buffer for a given file descriptor
+ *
+ * fd:      file descriptor
+ *
+ * returns: 1 - ok
+ *          0 - error, error code in errno
+ */
 {
     XrdFfsWcache_destroy(fd);
     fd -= XrdFfsPosix_baseFD;
@@ -115,8 +122,9 @@ int XrdFfsWcache_create(int fd)
     XrdFfsWcacheFbufs[fd].mlock = (pthread_mutex_t*)malloc(sizeof(pthread_mutex_t));
     if (XrdFfsWcacheFbufs[fd].mlock == NULL)
         return 0;
-    else
-        pthread_mutex_init(XrdFfsWcacheFbufs[fd].mlock, NULL);
+    errno = pthread_mutex_init(XrdFfsWcacheFbufs[fd].mlock, NULL);
+    if (errno)
+        return 0;
     return 1;
 }
 
@@ -161,6 +169,11 @@ ssize_t XrdFfsWcache_pwrite(int fd, char *buf, size_t len, off_t offset)
     ssize_t rc;
     char *bufptr;
     fd -= XrdFfsPosix_baseFD;
+    if (fd < 0)
+    {
+        errno = EBADF;
+        return -1;
+    }
 
 /* do not use caching under these cases */
     if (len > XrdFfsWcacheBufsize/2 || fd >= XrdFfsWcacheNFILES)

src/XrdFfs/XrdFfsXrootdfs.cc

@@ -715,21 +715,30 @@ static int xrootdfs_utimens(const char *path, const struct timespec ts[2])
 }
 
 static int xrootdfs_open(const char *path, struct fuse_file_info *fi)
+/*
+ * path:    path to file
+ * fi:      file info, return file descriptor in fi->fh
+ *
+ * returns 0 on success and -errno on error
+ */
 {
-    int res, lid = 1;
+    int fd, lid = 1;
     char rootpath[MAXROOTURLLEN]="";
     strncat(rootpath,xrootdfs.rdr, MAXROOTURLLEN - strlen(rootpath) -1);
     strncat(rootpath,path, MAXROOTURLLEN - strlen(rootpath) -1);
 
     XrdFfsMisc_xrd_secsss_register(fuse_get_context()->uid, fuse_get_context()->gid, &lid);
     XrdFfsMisc_xrd_secsss_editurl(rootpath, fuse_get_context()->uid, &lid);
-    res = XrdFfsPosix_open(rootpath, fi->flags, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
-    if (res == -1)
+    fd = XrdFfsPosix_open(rootpath, fi->flags, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
+    if (fd == -1)
         return -errno;
 
-    fi->fh = res;
-    XrdFfsWcache_create(fi->fh);
-    return 0;
+    fi->fh = fd;
+    // be careful, 0 means error for this function
+    if (XrdFfsWcache_create(fi->fh))
+        return 0;
+    else
+        return -errno;
 }
 
 static int xrootdfs_read(const char *path, char *buf, size_t size, off_t offset,