xfs
[Top] [All Lists]

[PATCH 2/6] xfs_fsr: fix leaks & catch error in fsrfile()

To: xfs@xxxxxxxxxxx
Subject: [PATCH 2/6] xfs_fsr: fix leaks & catch error in fsrfile()
From: Eric Sandeen <sandeen@xxxxxxxxxx>
Date: Fri, 1 Aug 2014 09:59:15 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1406905159-12415-1-git-send-email-sandeen@xxxxxxxxxx>
References: <1406905159-12415-1-git-send-email-sandeen@xxxxxxxxxx>
The allocated fshandlep leaks on most error paths;
restructure with an out: target that does all necessary
freeing, and initialize filehandles to -1 so that we
know whether they need to be closed on the error path.

While we're at it, if gettmpname() fails, we still
return 0 for an error, because error is initialized
to 0 and only set otherwise by fsrfile_common.
So if gettmpname() fails, we return success from the
function even though we did no work.  Fix that
as well by initializing error to -1.

Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
---
 fsr/xfs_fsr.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 48629fd..752d2db 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -809,15 +809,15 @@ fsrfile(char *fname, xfs_ino_t ino)
 {
        xfs_bstat_t     statbuf;
        jdm_fshandle_t  *fshandlep;
-       int     fd, fsfd;
-       int     error = 0;
+       int     fd = -1, fsfd = -1;
+       int     error = -1;
        char    *tname;
 
        fshandlep = jdm_getfshandle(getparent (fname) );
-       if (! fshandlep) {
+       if (!fshandlep) {
                fsrprintf(_("unable to construct sys handle for %s: %s\n"),
                        fname, strerror(errno));
-               return -1;
+               goto out;
        }
 
        /*
@@ -828,39 +828,39 @@ fsrfile(char *fname, xfs_ino_t ino)
        if (fsfd < 0) {
                fsrprintf(_("unable to open sys handle for %s: %s\n"),
                        fname, strerror(errno));
-               return -1;
+               goto out;
        }
 
        if ((xfs_bulkstat_single(fsfd, &ino, &statbuf)) < 0) {
                fsrprintf(_("unable to get bstat on %s: %s\n"),
                        fname, strerror(errno));
-               close(fsfd);
-               return -1;
+               goto out;
        }
 
        fd = jdm_open(fshandlep, &statbuf, O_RDWR|O_DIRECT);
        if (fd < 0) {
                fsrprintf(_("unable to open handle %s: %s\n"),
                        fname, strerror(errno));
-               close(fsfd);
-               return -1;
+               goto out;
        }
 
        /* Get the fs geometry */
        if (xfs_getgeom(fsfd, &fsgeom) < 0 ) {
                fsrprintf(_("Unable to get geom on fs for: %s\n"), fname);
-               close(fsfd);
-               return -1;
+               goto out;
        }
 
-       close(fsfd);
-
        tname = gettmpname(fname);
 
        if (tname)
                error = fsrfile_common(fname, tname, NULL, fd, &statbuf);
 
-       close(fd);
+out:
+       if (fsfd >= 0)
+               close(fsfd);
+       if (fd >= 0)
+               close(fd);
+       free(fshandlep);
 
        return error;
 }
-- 
1.7.1

<Prev in Thread] Current Thread [Next in Thread>