From 80a2d2752fcc363cba04a7421451cbe000edd6c5 Mon Sep 17 00:00:00 2001 From: Csaba Henk Date: Wed, 16 May 2012 15:40:47 +0530 Subject: 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 Reviewed-on: http://review.gluster.com/3345 Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- xlators/features/marker/utils/src/gsyncd.c | 63 +++++++-------------------- xlators/features/marker/utils/src/procdiggy.c | 59 +++++++++---------------- 2 files changed, 36 insertions(+), 86 deletions(-) (limited to 'xlators/features') 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; } -- cgit