Fix unchecked return values
authorJeffrey Hutzelman <jhutz@cmu.edu>
Sun, 16 Jun 2013 19:28:03 +0000 (15:28 -0400)
committerJeffrey Altman <jaltman@your-file-system.com>
Tue, 6 Jan 2015 16:41:57 +0000 (11:41 -0500)
This change fixes numerous places where the return values of various
system calls and standard library routines are not checked.  In
particular, this fixes occurrances called out when building on Ubuntu
12.10, with gcc 4.7.2 and eglibc 2.15-0ubuntu20.1, when the possible
failure is one we actually do (or should) care about.  This change
does not consider calls where the failure is one we deliberately
choose to ignore.

Change-Id: Id526f5dd7ee48be2604b77a3f00ea1e51b08c21d
Reviewed-on: http://gerrit.openafs.org/9979
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>

16 files changed:
src/auth/cellconfig.c
src/bozo/bosserver.c
src/budb/db_text.c
src/kauth/kkids.c
src/lwp/lwp.c
src/ptserver/pt_util.c
src/ptserver/ptubik.c
src/sys/rmtsysc.c
src/venus/test/fulltest.c
src/venus/up.c
src/viced/afsfileprocs.c
src/viced/state_analyzer.c
src/viced/viced.c
src/vlserver/cnvldb.c
src/vol/namei_ops.c
src/volser/restorevol.c

index 7f7aaaf..2b99c0b 100644 (file)
@@ -453,7 +453,7 @@ afsconf_Open(const char *adir)
            /* The "AFSCONF" environment (or contents of "/.AFSCONF") will be typically set to something like "/afs/<cell>/common/etc" where, by convention, the default files for "ThisCell" and "CellServDB" will reside; note that a major drawback is that a given afs client on that cell may NOT contain the same contents... */
            char *home_dir;
            afsconf_FILE *fp;
-           size_t len;
+           size_t len = 0;
            int r;
 
            if (!(home_dir = getenv("HOME"))) {
@@ -462,8 +462,6 @@ afsconf_Open(const char *adir)
                if (fp == 0)
                    goto fail;
 
-               fgets(afs_confdir, 128, fp);
-               fclose(fp);
            } else {
                char *pathname = NULL;
 
@@ -480,10 +478,10 @@ afsconf_Open(const char *adir)
                    if (fp == 0)
                        goto fail;
                }
-               fgets(afs_confdir, 128, fp);
-               fclose(fp);
            }
-           len = strlen(afs_confdir);
+           if (fgets(afs_confdir, 128, fp) != NULL)
+               len = strlen(afs_confdir);
+           fclose(fp);
            if (len == 0)
                goto fail;
 
index 07f966e..ca85c22 100644 (file)
@@ -253,32 +253,46 @@ CreateDirs(const char *coredir)
        (!strncmp
         (AFSDIR_USR_DIRPATH, AFSDIR_SERVER_BIN_DIRPATH,
          strlen(AFSDIR_USR_DIRPATH)))) {
-       MakeDir(AFSDIR_USR_DIRPATH);
+       if (MakeDir(AFSDIR_USR_DIRPATH))
+           return errno;
     }
     if (!strncmp
        (AFSDIR_SERVER_AFS_DIRPATH, AFSDIR_SERVER_BIN_DIRPATH,
         strlen(AFSDIR_SERVER_AFS_DIRPATH))) {
-       MakeDir(AFSDIR_SERVER_AFS_DIRPATH);
+       if (MakeDir(AFSDIR_SERVER_AFS_DIRPATH))
+           return errno;
     }
-    MakeDir(AFSDIR_SERVER_BIN_DIRPATH);
-    MakeDir(AFSDIR_SERVER_ETC_DIRPATH);
-    MakeDir(AFSDIR_SERVER_LOCAL_DIRPATH);
-    MakeDir(AFSDIR_SERVER_DB_DIRPATH);
-    MakeDir(AFSDIR_SERVER_LOGS_DIRPATH);
+    if (MakeDir(AFSDIR_SERVER_BIN_DIRPATH))
+       return errno;
+    if (MakeDir(AFSDIR_SERVER_ETC_DIRPATH))
+       return errno;
+    if (MakeDir(AFSDIR_SERVER_LOCAL_DIRPATH))
+       return errno;
+    if (MakeDir(AFSDIR_SERVER_DB_DIRPATH))
+       return errno;
+    if (MakeDir(AFSDIR_SERVER_LOGS_DIRPATH))
+       return errno;
 #ifndef AFS_NT40_ENV
     if (!strncmp
        (AFSDIR_CLIENT_VICE_DIRPATH, AFSDIR_CLIENT_ETC_DIRPATH,
         strlen(AFSDIR_CLIENT_VICE_DIRPATH))) {
-       MakeDir(AFSDIR_CLIENT_VICE_DIRPATH);
+       if (MakeDir(AFSDIR_CLIENT_VICE_DIRPATH))
+           return errno;
     }
-    MakeDir(AFSDIR_CLIENT_ETC_DIRPATH);
+    if (MakeDir(AFSDIR_CLIENT_ETC_DIRPATH))
+       return errno;
 
-    symlink(AFSDIR_SERVER_THISCELL_FILEPATH, AFSDIR_CLIENT_THISCELL_FILEPATH);
-    symlink(AFSDIR_SERVER_CELLSERVDB_FILEPATH,
-           AFSDIR_CLIENT_CELLSERVDB_FILEPATH);
+    if (symlink(AFSDIR_SERVER_THISCELL_FILEPATH,
+           AFSDIR_CLIENT_THISCELL_FILEPATH))
+       return errno;
+    if (symlink(AFSDIR_SERVER_CELLSERVDB_FILEPATH,
+           AFSDIR_CLIENT_CELLSERVDB_FILEPATH))
+       return errno;
 #endif /* AFS_NT40_ENV */
-    if (coredir)
-       MakeDir(coredir);
+    if (coredir) {
+       if (MakeDir(coredir))
+           return errno;
+    }
     return 0;
 }
 
@@ -975,21 +989,32 @@ main(int argc, char **argv, char **envp)
      */
 
 #ifndef AFS_NT40_ENV
-    if (!nofork)
-       daemon(1, 0);
+    if (!nofork) {
+       if (daemon(1, 0))
+           printf("bosserver: warning - daemon() returned code %d\n", errno);
+    }
 #endif /* ! AFS_NT40_ENV */
 
     /* create useful dirs */
-    CreateDirs(DoCore);
+    i = CreateDirs(DoCore);
+    if (i) {
+       printf("bosserver: could not set up directories, code %d\n", i);
+       exit(1);
+    }
 
     /* Write current state of directory permissions to log file */
     DirAccessOK();
 
     /* chdir to AFS log directory */
     if (DoCore)
-       chdir(DoCore);
+       i = chdir(DoCore);
     else
-       chdir(AFSDIR_SERVER_LOGS_DIRPATH);
+       i = chdir(AFSDIR_SERVER_LOGS_DIRPATH);
+    if (i) {
+       printf("bosserver: could not change to %s, code %d\n",
+              DoCore ? DoCore : AFSDIR_SERVER_LOGS_DIRPATH, errno);
+       exit(1);
+    }
 
     /* try to read the key from the config file */
     tdir = afsconf_Open(AFSDIR_SERVER_ETC_DIRPATH);
index 1d1a3b4..e88ef51 100644 (file)
@@ -457,22 +457,28 @@ saveTextToFile(struct ubik_trans *ut, struct textBlock *tbPtr)
     afs_int32 blockAddr;
     struct block block;
     char filename[128];
-    afs_int32 size, chunkSize;
+    afs_int32 size, totalSize, chunkSize;
     int fid;
 
     sprintf(filename, "%s/%s", gettmpdir(), "dbg_XXXXXX");
 
     fid = mkstemp(filename);
-    size = ntohl(tbPtr->size);
+    totalSize = size = ntohl(tbPtr->size);
     blockAddr = ntohl(tbPtr->textAddr);
     while (size) {
        chunkSize = min(BLOCK_DATA_SIZE, size);
        dbread(ut, blockAddr, (char *)&block, sizeof(block));
-       write(fid, &block.a[0], chunkSize);
+       if (write(fid, &block.a[0], chunkSize) < 0)
+           break;
        blockAddr = ntohl(block.h.next);
        size -= chunkSize;
     }
     close(fid);
-    printf("wrote debug file %s\n", filename);
+    if (size) {
+       printf("Wrote partial debug file (%ld bytes out of %ld)\n",
+              (long)(totalSize - size), (long)totalSize);
+    } else {
+       printf("wrote debug file %s\n", filename);
+    }
 }
 
index 90d9b1e..979e544 100644 (file)
@@ -387,8 +387,10 @@ init_child(char *myname)
      * reads from the latter, the child reads from the former, and
      * writes to the latter.
      */
-    pipe(pipe1);
-    pipe(pipe2);
+    if (pipe(pipe1) || pipe(pipe2)) {
+       using_child = 0;
+       return 0;
+    }
 
     /* fork a child */
     pid = fork();
@@ -429,7 +431,8 @@ password_bad(char *pw)
     if (using_child) {
        fprintf(childin, "%s\n", pw);
        fflush(childin);
-       fscanf(childout, "%d", &rc);
+       if (fscanf(childout, "%d", &rc) < 1)
+           rc = -1;
     }
 
     return (rc);
index e521ff4..50228b4 100644 (file)
@@ -979,11 +979,15 @@ Overflow_Complain(void)
     currenttime = time(0);
     timeStamp = ctime(&currenttime);
     timeStamp[24] = 0;
-    write(2, timeStamp, strlen(timeStamp));
-
-    write(2, msg1, strlen(msg1));
-    write(2, lwp_cpptr->name, strlen(lwp_cpptr->name));
-    write(2, msg2, strlen(msg2));
+    if (write(2, timeStamp, strlen(timeStamp)) < 0)
+       return;
+
+    if (write(2, msg1, strlen(msg1)) < 0)
+       return;
+    if (write(2, lwp_cpptr->name, strlen(lwp_cpptr->name)) < 0)
+       return;
+    if (write(2, msg2, strlen(msg2)) < 0)
+       return;
 }
 
 static void
index 0fcfbe1..00b7dea 100644 (file)
@@ -394,7 +394,11 @@ static int
 display_entry(int offset)
 {
     lseek(dbase_fd, offset + HDRSIZE, L_SET);
-    read(dbase_fd, &pre, sizeof(struct prentry));
+    if (read(dbase_fd, &pre, sizeof(struct prentry)) < 0) {
+       fprintf(stderr, "pt_util: error reading entry %d: %s\n",
+               offset, strerror(errno));
+       exit(1);
+    }
 
     fix_pre(&pre);
 
@@ -493,7 +497,11 @@ display_group(int id)
        offset = pre.next;
        while (offset) {
            lseek(dbase_fd, offset + HDRSIZE, L_SET);
-           read(dbase_fd, &prco, sizeof(struct contentry));
+           if (read(dbase_fd, &prco, sizeof(struct contentry)) < 0) {
+               fprintf(stderr, "pt_util: read i/o error: %s\n",
+                       strerror(errno));
+               exit(1);
+           }
            prco.next = ntohl(prco.next);
            for (i = 0; i < COSIZE; i++) {
                prco.entries[i] = ntohl(prco.entries[i]);
index f059d2c..9d2d916 100644 (file)
@@ -31,6 +31,7 @@ ubik_BeginTrans(struct ubik_dbase *dbase, afs_int32 transMode,
 {
     static int init = 0;
     struct ubik_hdr thdr;
+    ssize_t count;
 
     if (!init) {
        memset(&thdr, 0, sizeof(thdr));
@@ -38,9 +39,15 @@ ubik_BeginTrans(struct ubik_dbase *dbase, afs_int32 transMode,
        thdr.version.counter = htonl(0);
        thdr.magic = htonl(UBIK_MAGIC);
        thdr.size = htons(HDRSIZE);
-       lseek(dbase_fd, 0, 0);
-       write(dbase_fd, &thdr, sizeof(thdr));
-       fsync(dbase_fd);
+       if (lseek(dbase_fd, 0, 0) == (off_t)-1)
+           return errno;
+       count = write(dbase_fd, &thdr, sizeof(thdr));
+       if (count < 0)
+           return errno;
+       else if (count != sizeof(thdr))
+           return UIOERROR;
+       if (fsync(dbase_fd))
+           return errno;
        init = 1;
     }
     return (0);
index 0893a98..a66eb88 100644 (file)
@@ -57,7 +57,7 @@ GetAfsServerAddr(char *syscall)
     if (!(afs_server = getenv("AFSSERVER"))) {
        char *home_dir;
        FILE *fp;
-       int len;
+       int len = 0;
 
        if (!(home_dir = getenv("HOME"))) {
            /* Our last chance is the "/.AFSSERVER" file */
@@ -65,8 +65,6 @@ GetAfsServerAddr(char *syscall)
            if (fp == 0) {
                return 0;
            }
-           fgets(server_name, 128, fp);
-           fclose(fp);
        } else {
            char *pathname;
 
@@ -83,10 +81,10 @@ GetAfsServerAddr(char *syscall)
                    return 0;
                }
            }
-           fgets(server_name, 128, fp);
-           fclose(fp);
        }
-       len = strlen(server_name);
+       if (fgets(server_name, 128, fp) != NULL)
+           len = strlen(server_name);
+       fclose(fp);
        if (len == 0) {
            return 0;
        }
index dda9d9a..e3b6d9b 100644 (file)
@@ -233,7 +233,7 @@ main(int argc, char **argv)
        perror("open ronly");
        return -1;
     }
-    fchown(fd1, 1, -1);                /* don't check error code, may fail on Ultrix */
+    code = fchown(fd1, 1, -1); /* don't check error code, may fail on Ultrix */
     code = write(fd1, "test", 4);
     if (code != 4) {
        printf("rotest short read (%d)\n", code);
@@ -296,7 +296,10 @@ main(int argc, char **argv)
     }
 
     /* now finish up */
-    chdir("..");
+    if (chdir("..") < 0) {
+       perror("chdir ..");
+       return -1;
+    }
     rmdir(dirName);
     printf("Test completed successfully.\n");
     return 0;
index 9334f0f..31f0d77 100644 (file)
@@ -184,8 +184,10 @@ MakeParent(char *file, afs_int32 owner)
            fflush(stdout);
        }
 
-       mkdir(parent, 0777);
-       chown(parent, owner, -1);
+       if (mkdir(parent, 0777))
+           return (0);
+       if (chown(parent, owner, -1))
+           return (0);
     }
     return (1);
 }                              /*MakeParent */
index 8209ab6..914024e 100644 (file)
@@ -6554,13 +6554,17 @@ StoreData_RXStyle(Volume * volptr, Vnode * targetptr, struct AFSFid * Fid,
             (afs_uintmax_t) Pos, (afs_uintmax_t) DataLength,
             (afs_uintmax_t) FileLength, (afs_uintmax_t) Length));
 
-    /* truncate the file iff it needs it (ftruncate is slow even when its a noop) */
-    if (FileLength < DataLength)
-       FDH_TRUNC(fdP, FileLength);
     bytesTransfered = 0;
 #ifndef HAVE_PIOV
     tbuffer = AllocSendBuffer();
 #endif /* HAVE_PIOV */
+    /* truncate the file iff it needs it (ftruncate is slow even when its a noop) */
+    if (FileLength < DataLength) {
+       errorCode = FDH_TRUNC(fdP, FileLength);
+       if (errorCode)
+           goto done;
+    }
+
     /* if length == 0, the loop below isn't going to do anything, including
      * extend the length of the inode, which it must do, since the file system
      * assumes that the inode length == vnode's file length.  So, we extend
index 184a9c8..6d8b209 100644 (file)
@@ -330,7 +330,8 @@ prompt(void)
                fprintf(stderr, "prompt state broken; aborting\n");
                return;
            }
-           fgets(input, 256, stdin);
+           if (fgets(input, 256, stdin) == NULL)
+               return;
 
            if (!strcmp(input, "")) {
                /* repeat last command */
index 6072647..7cc7aaa 100644 (file)
@@ -365,18 +365,18 @@ char adminName[MAXADMINNAME];
 static void
 CheckAdminName(void)
 {
-    int fd = 0;
+    int fd = -1;
     struct afs_stat status;
 
     if ((afs_stat("/AdminName", &status)) ||   /* if file does not exist */
        (status.st_size <= 0) ||        /* or it is too short */
        (status.st_size >= (MAXADMINNAME)) ||   /* or it is too long */
-       !(fd = afs_open("/AdminName", O_RDONLY, 0))) {  /* or the open fails */
+       (fd = afs_open("/AdminName", O_RDONLY, 0)) < 0 || /* or open fails */
+       read(fd, adminName, status.st_size) != status.st_size) { /* or read */
+
        strcpy(adminName, "System:Administrators");     /* use the default name */
-    } else {
-       (void)read(fd, adminName, status.st_size);      /* use name from the file */
     }
-    if (fd)
+    if (fd >= 0)
        close(fd);              /* close fd if it was opened */
 
 }                              /*CheckAdminName */
index d64c086..ca42080 100644 (file)
@@ -72,6 +72,8 @@ static int
 handleit(struct cmd_syndesc *as, void *arock)
 {
     int w, old, new, rc, dump = 0, fromv = 0;
+    ssize_t count;
+
     char ubik[80];             /* space for some ubik header */
     union {
        struct vlheader_1 header1;
@@ -101,8 +103,18 @@ handleit(struct cmd_syndesc *as, void *arock)
     }
 
     /* Read the version */
-    lseek(old, 64, L_SET);
-    read(old, &fromv, sizeof(int));
+    if (lseek(old, 64, L_SET) == (off_t)-1) {
+       perror(pn);
+       exit(-1);
+    }
+    count = read(old, &fromv, sizeof(int));
+    if (count < 0) {
+       perror(pn);
+       exit(-1);
+    } else if (count != sizeof(int)) {
+       fprintf(stderr, "%s: Premature EOF reading database version.\n", pn);
+       exit(-1);
+    }
     fromv = ntohl(fromv);
     if ((fromv < 1) || (fromv > 4)) {
        fprintf(stderr, "%s", pn);
@@ -111,8 +123,18 @@ handleit(struct cmd_syndesc *as, void *arock)
     }
 
     /* Sequentially read the database converting the entries as we go */
-    lseek(old, 0, L_SET);
-    read(old, ubik, 64);
+    if (lseek(old, 0, L_SET) == (off_t)-1) {
+       perror(pn);
+       exit(-1);
+    }
+    count = read(old, ubik, 64);
+    if (count < 0) {
+       perror(pn);
+       exit(-1);
+    } else if (count != 64) {
+       fprintf(stderr, "%s: Premature EOF reading database header.\n", pn);
+       exit(-1);
+    }
     readheader(old, fromv, &oldheader);
     if (fromv == 1) {
        dbsize = ntohl(oldheader.header1.vital_header.eofPtr);
index fc0a14a..19aea95 100644 (file)
@@ -326,7 +326,7 @@ int
 namei_ViceREADME(char *partition)
 {
     char filename[32];
-    int fd;
+    int fd, len, e = 0;
 
     /* Create the inode directory if we're starting for the first time */
     snprintf(filename, sizeof filename, "%s" OS_DIRSEP "%s", partition,
@@ -338,8 +338,12 @@ namei_ViceREADME(char *partition)
              partition, INODEDIR);
     fd = OS_OPEN(filename, O_WRONLY | O_CREAT | O_TRUNC, 0444);
     if (fd != INVALID_FD) {
-       (void)OS_WRITE(fd, VICE_README, strlen(VICE_README));
+       len = strlen(VICE_README);
+       if (OS_WRITE(fd, VICE_README, len) != len)
+           e = errno;
        OS_CLOSE(fd);
+       if (e)
+           errno = e;
     }
     return (errno);
 }
@@ -1568,7 +1572,10 @@ namei_GetLinkCount(FdHandle_t * h, Inode ino, int lockit, int fixup, int nowrite
            NAMEI_GLC_UNLOCK;
            goto bad_getLinkByte;
        }
-        FDH_TRUNC(h, offset+sizeof(row));
+        if (FDH_TRUNC(h, offset+sizeof(row))) {
+           NAMEI_GLC_UNLOCK;
+           goto bad_getLinkByte;
+       }
         row = 1 << index;
        rc = FDH_PWRITE(h, (char *)&row, sizeof(row), offset);
        NAMEI_GLC_UNLOCK;
@@ -3165,7 +3172,11 @@ namei_ConvertROtoRWvolume(char *pname, VolumeId volumeId)
 #ifdef AFS_NT40_ENV
     MoveFileEx(n.n_path, newpath, MOVEFILE_WRITE_THROUGH);
 #else
-    link(newpath, n.n_path);
+    if (link(newpath, n.n_path)) {
+       Log("1 namei_ConvertROtoRWvolume: could not move SmallIndex file: %s\n", n.n_path);
+       code = -1;
+       goto done;
+    }
     OS_UNLINK(newpath);
 #endif
 
@@ -3184,7 +3195,11 @@ namei_ConvertROtoRWvolume(char *pname, VolumeId volumeId)
 #ifdef AFS_NT40_ENV
     MoveFileEx(n.n_path, newpath, MOVEFILE_WRITE_THROUGH);
 #else
-    link(newpath, n.n_path);
+    if (link(newpath, n.n_path)) {
+       Log("1 namei_ConvertROtoRWvolume: could not move LargeIndex file: %s\n", n.n_path);
+       code = -1;
+       goto done;
+    }
     OS_UNLINK(newpath);
 #endif
 
index aef5627..0f28875 100644 (file)
@@ -653,6 +653,7 @@ ReadVNode(afs_int32 count)
                int fid;
                int lfile;
                afs_sfsize_t size, s;
+               ssize_t count;
 
                /* Check if its vnode-file-link exists. If not,
                 * then the file will be an orphaned file.
@@ -676,14 +677,14 @@ ReadVNode(afs_int32 count)
 
                /* Write the file out */
                fid = open(filename, (O_CREAT | O_WRONLY | O_TRUNC), mode);
+               if (fid < 0) {
+                   fprintf(stderr, "Open %s: Errno = %d\n", filename, errno);
+                   goto open_fail;
+               }
                size = vn.dataSize;
                while (size > 0) {
                    s = (afs_int32) ((size > BUFSIZE) ? BUFSIZE : size);
                    code = fread(buf, 1, s, dumpfile);
-                   if (code > 0) {
-                       (void)write(fid, buf, code);
-                       size -= code;
-                   }
                    if (code != s) {
                        if (code < 0)
                            fprintf(stderr, "Code = %d; Errno = %d\n", code,
@@ -698,6 +699,20 @@ ReadVNode(afs_int32 count)
                        }
                        break;
                    }
+                   if (code > 0) {
+                       count = write(fid, buf, code);
+                       if (count < 0) {
+                           fprintf(stderr, "Count = %ld, Errno = %d\n",
+                                   (long)count, errno);
+                           break;
+                       } else if (count != code) {
+                           fprintf(stderr, "Wrote %llu bytes out of %llu\n",
+                                   (afs_uintmax_t) count,
+                                   (afs_uintmax_t) code);
+                           break;
+                       }
+                       size -= code;
+                   }
                }
                close(fid);
                if (size != 0) {
@@ -705,6 +720,7 @@ ReadVNode(afs_int32 count)
                            filename, fname);
                }
 
+open_fail:
                /* Remove the link to the file */
                if (lfile) {
                    unlink(filename);