diff options
author | Csaba Henk <csaba@redhat.com> | 2012-05-16 15:40:47 +0530 |
---|---|---|
committer | Vijay Bellur <vijay@gluster.com> | 2012-05-18 00:45:15 -0700 |
commit | 80a2d2752fcc363cba04a7421451cbe000edd6c5 (patch) | |
tree | 17c7ff44f0ba7c21f5b0409387d8624b22f0d29d | |
parent | 8c7e1cfdebc6591bbedbe2e1b98b7b3279e6519c (diff) |
geo-rep / gsyncd.c: fix coverity fix
gsyncd wrapper was segfaulting as coverity fix freed
up pointer at wrong place (after it was reused)
Instead of the apporach of the original coverity fix
that added elaborate control flow to hunt down potential
leaks, here we move the code over to static allocations
in place of (the not really necessary) dynamic ones.
Change-Id: Ida3855ff4a4f4371b350d27f858f129ceed51785
BUG: 789278
Signed-off-by: Csaba Henk <csaba@redhat.com>
Reviewed-on: http://review.gluster.com/3345
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vijay@gluster.com>
-rw-r--r-- | xlators/features/marker/utils/src/gsyncd.c | 63 | ||||
-rw-r--r-- | xlators/features/marker/utils/src/procdiggy.c | 59 |
2 files changed, 36 insertions, 86 deletions
diff --git a/xlators/features/marker/utils/src/gsyncd.c b/xlators/features/marker/utils/src/gsyncd.c index b5263c7287c..9c598ce668e 100644 --- a/xlators/features/marker/utils/src/gsyncd.c +++ b/xlators/features/marker/utils/src/gsyncd.c @@ -161,6 +161,7 @@ static int find_gsyncd (pid_t pid, pid_t ppid, char *name, void *data) { char buf[NAME_MAX * 2] = {0,}; + char path[PATH_MAX] = {0,}; char *p = NULL; int zeros = 0; int ret = 0; @@ -168,28 +169,16 @@ find_gsyncd (pid_t pid, pid_t ppid, char *name, void *data) pid_t *pida = (pid_t *)data; if (ppid != pida[0]) - goto out; + return 0; - ret = gf_asprintf (&p, PROC"/%d/cmdline", pid); - if (ret == -1) { - fprintf (stderr, "out of memory\n"); - goto out; - } - - ret = 0; - - fd = open (p, O_RDONLY); + sprintf (path, PROC"/%d/cmdline", pid); + fd = open (path, O_RDONLY); if (fd == -1) - goto out; - + return 0; ret = read (fd, buf, sizeof (buf)); close (fd); - - if (ret == -1) { - ret = 0; - goto out; - } - + if (ret == -1) + return 0; for (zeros = 0, p = buf; zeros < 2 && p < buf + ret; p++) zeros += !*p; @@ -210,25 +199,19 @@ find_gsyncd (pid_t pid, pid_t ppid, char *name, void *data) if (ret == 1) { if (pida[1] != -1) { fprintf (stderr, GSYNCD_PY" sibling is not unique"); - ret = -1; - goto out; + return -1; } pida[1] = pid; } - ret = 0; -out: - if (p) - GF_FREE (p); - - return ret; + return 0; } static int invoke_rsync (int argc, char **argv) { int i = 0; - char *p = NULL; + char path[PATH_MAX] = {0,}; pid_t pid = -1; pid_t ppid = -1; pid_t pida[] = {-1, -1}; @@ -255,10 +238,11 @@ invoke_rsync (int argc, char **argv) fprintf (stderr, "sshd ancestor not found\n"); goto error; } - if (strcmp (name, "sshd") == 0) + if (strcmp (name, "sshd") == 0) { + GF_FREE (name); break; + } GF_FREE (name); - name = NULL; } /* look up "ssh-sibling" gsyncd */ pida[0] = pid; @@ -268,15 +252,12 @@ invoke_rsync (int argc, char **argv) goto error; } /* check if rsync target matches gsyncd target */ - if (gf_asprintf (&p, PROC"/%d/cwd", pida[1]) == -1) { - fprintf (stderr, "out of memory\n"); - goto error; - } - ret = readlink (p, buf, sizeof (buf)); + sprintf (path, PROC"/%d/cwd", pida[1]); + ret = readlink (path, buf, sizeof (buf)); if (ret == -1 || ret == sizeof (buf)) goto error; if (strcmp (argv[argc - 1], "/") == 0 /* root dir cannot be a target */ || - (strcmp (argv[argc - 1], p) /* match against gluster target */ && + (strcmp (argv[argc - 1], path) /* match against gluster target */ && strcmp (argv[argc - 1], buf) /* match against file target */) != 0) { fprintf (stderr, "rsync target does not match "GEOREP" session\n"); goto error; @@ -286,22 +267,10 @@ invoke_rsync (int argc, char **argv) execvp (RSYNC, argv); - if (p) - GF_FREE (p); - - if (name) - GF_FREE (name); - fprintf (stderr, "exec of "RSYNC" failed\n"); return 127; error: - if (p) - GF_FREE (p); - - if (name) - GF_FREE (name); - fprintf (stderr, "disallowed "RSYNC" invocation\n"); return 1; } diff --git a/xlators/features/marker/utils/src/procdiggy.c b/xlators/features/marker/utils/src/procdiggy.c index 18296a47945..0baab966dc5 100644 --- a/xlators/features/marker/utils/src/procdiggy.c +++ b/xlators/features/marker/utils/src/procdiggy.c @@ -38,20 +38,15 @@ pidinfo (pid_t pid, char **name) { char buf[NAME_MAX * 2] = {0,}; FILE *f = NULL; + char path[PATH_MAX] = {0,}; char *p = NULL; - char *free_p = NULL; int ret = 0; - ret = gf_asprintf (&p, PROC"/%d/status", pid); - if (ret == -1) - goto oom; + sprintf (path, PROC"/%d/status", pid); - f = fopen (p, "r"); - if (!f) { - GF_FREE (p); + f = fopen (path, "r"); + if (!f) return -1; - } - free_p = p; if (name) *name = NULL; @@ -69,8 +64,10 @@ pidinfo (pid_t pid, char **name) if (p) { while (isspace (*++p)); *name = gf_strdup (p); - if (!*name) - goto oom; + if (!*name) { + pid = -2; + goto out; + } continue; } } @@ -86,17 +83,12 @@ pidinfo (pid_t pid, char **name) pid = -1; out: - if (free_p) - GF_FREE (free_p); fclose (f); + if (pid == -1 && name && *name) + GF_FREE (name); + if (pid == -2) + fprintf (stderr, "out of memory\n"); return pid; - - oom: - if (free_p) - GF_FREE (free_p); - fclose (f); - fprintf (stderr, "out of memory\n"); - return -2; } int @@ -111,38 +103,27 @@ prociter (int (*proch) (pid_t pid, pid_t ppid, char *tmpname, void *data), int ret = 0; d = opendir (PROC); - if (!d) { - ret = -1; - goto out; - } + if (!d) + return -1; while (errno = 0, de = readdir (d)) { if (gf_string2int (de->d_name, &pid) != -1 && pid >= 0) { ppid = pidinfo (pid, &name); switch (ppid) { case -1: continue; - case -2: closedir (d); return -1; + case -2: ret = -1; break; } ret = proch (pid, ppid, name, data); - if (ret) { - goto out; - } GF_FREE (name); - name = NULL; + if (ret) + break; } } - if (errno) { + closedir (d); + if (!de && errno) { fprintf (stderr, "failed to traverse "PROC" (%s)\n", strerror (errno)); - goto out; + ret = -1; } - ret = 0; -out: - if (d) - closedir (d); - - if (name) - GF_FREE (name); - return ret; } |