Date: Thu, 29 Aug 2002 21:33:11 -0500 (CDT) From: "Chris S.J. Peron" <maneo@bsdpro.com> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/42179: [patch] fetch: memory leak and general optimizations Message-ID: <200208300233.g7U2XB7F023811@xor.sqrt.ca>
next in thread | raw e-mail | index | archive | help
>Number: 42179
>Category: bin
>Synopsis: [patch] fetch: memory leak and general optimizations
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Aug 29 19:40:01 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator: Chris S.J. Peron
>Release: FreeBSD 4.6-STABLE i386
>Organization:
>Environment:
System: FreeBSD xor.sqrt.ca 4.6-STABLE FreeBSD 4.6-STABLE #3: Tue Aug 13 17:17:48 CDT 2002 cperon@xor.sqrt.ca:/usr/src/sys/compile/opcode i386
>Description:
main allocates a transfer buffer that is never free'ed when fetch
is done with it.
Currently, fetch has used the integer storage class for essentially
all boolean variables. (16 which takes up 16*5 bytes (64)) when it could be
using a single integer and unique bitmasks.
>How-To-Repeat:
N/A
>Fix:
--- /usr/src/usr.bin/fetch/fetch.c.leaks Thu Aug 29 09:51:43 2002
+++ /usr/src/usr.bin/fetch/fetch.c Thu Aug 29 21:14:43 2002
@@ -48,32 +48,36 @@
#define MINBUFSIZE 4096
-/* Option flags */
-int A_flag; /* -A: do not follow 302 redirects */
-int a_flag; /* -a: auto retry */
+/*
+ * bit masks for command line options.
+ */
+#define FL_A (1 << 0) /* -A: do not follow 302 redirects */
+#define FL_a (1 << 1) /* -a: auto retry */
+#define FL_b (1 << 2) /*! -b: workaround TCP bug */
+#define FL_d (1 << 3) /* -d: direct connection */
+#define FL_F (1 << 4) /* -F: restart without checking mtime */
+#define FL_l (1 << 5) /* -l: link rather than copy file: URLs */
+#define FL_o (1 << 6) /* -o: specify output file */
+#define FL_m (1 << 7) /* -[Mm]: mirror mode */
+#define FL_n (1 << 8) /* -n: do not preserve modification time */
+#define FL_p (1 << 9) /* -[Pp]: use passive FTP */
+#define FL_R (1 << 10) /* -R: don't delete partially transferred files */
+#define FL_r (1 << 11) /* -r: restart previously interrupted transfer */
+#define FL_s (1 << 12) /* -s: show size, don't fetch */
+#define FL_t (1 << 13) /*! -t: workaround TCP bug */
+#define FL_U (1 << 14) /* -U: do not use high ports */
+#define FL_1 (1 << 15) /* -1: stop at first successful file */
+
+u_int cmd_opts = 0; /* bit field for command line options */
off_t B_size; /* -B: buffer size */
-int b_flag; /*! -b: workaround TCP bug */
char *c_dirname; /* -c: remote directory */
-int d_flag; /* -d: direct connection */
-int F_flag; /* -F: restart without checking mtime */
char *f_filename; /* -f: file to fetch */
char *h_hostname; /* -h: host to fetch from */
-int l_flag; /* -l: link rather than copy file: URLs */
-int m_flag; /* -[Mm]: mirror mode */
-int n_flag; /* -n: do not preserve modification time */
-int o_flag; /* -o: specify output file */
int o_directory; /* output file is a directory */
char *o_filename; /* name of output file */
int o_stdout; /* output file is stdout */
-int once_flag; /* -1: stop at first successful file */
-int p_flag; /* -[Pp]: use passive FTP */
-int R_flag; /* -R: don't delete partially transferred files */
-int r_flag; /* -r: restart previously interrupted transfer */
off_t S_size; /* -S: require size to match */
-int s_flag; /* -s: show size, don't fetch */
u_int T_secs = 120; /* -T: transfer timeout in seconds */
-int t_flag; /*! -t: workaround TCP bug */
-int U_flag; /* -U: do not use high ports */
int v_level = 1; /* -v: verbosity level */
int v_tty; /* stdout is a tty */
pid_t pgrp; /* our process group */
@@ -304,20 +308,20 @@
/* FTP specific flags */
if (strcmp(url->scheme, "ftp") == 0) {
- if (p_flag)
+ if (cmd_opts & FL_p)
strcat(flags, "p");
- if (d_flag)
+ if (cmd_opts & FL_d)
strcat(flags, "d");
- if (U_flag)
+ if (cmd_opts & FL_U)
strcat(flags, "l");
timeout = T_secs ? T_secs : ftp_timeout;
}
/* HTTP specific flags */
if (strcmp(url->scheme, "http") == 0) {
- if (d_flag)
+ if (cmd_opts & FL_d)
strcat(flags, "d");
- if (A_flag)
+ if (cmd_opts & FL_A)
strcat(flags, "A");
timeout = T_secs ? T_secs : http_timeout;
}
@@ -326,7 +330,7 @@
fetchTimeout = timeout;
/* just print size */
- if (s_flag) {
+ if (cmd_opts & FL_s) {
if (fetchStat(url, &us, flags) == -1)
goto failure;
if (us.size == -1)
@@ -354,7 +358,7 @@
warnx("%s: stat()", path);
goto failure;
}
- if (!o_stdout && r_flag && S_ISREG(sb.st_mode))
+ if (!o_stdout && (cmd_opts & FL_r) && S_ISREG(sb.st_mode))
url->offset = sb.st_size;
/* start the transfer */
@@ -378,7 +382,7 @@
}
/* symlink instead of copy */
- if (l_flag && strcmp(url->scheme, "file") == 0 && !o_stdout) {
+ if ((cmd_opts & FL_l) && strcmp(url->scheme, "file") == 0 && !o_stdout) {
if (symlink(url->doc, path) == -1) {
warn("%s: symlink()", path);
goto failure;
@@ -401,13 +405,13 @@
if (o_stdout) {
/* output to stdout */
of = stdout;
- } else if (r_flag && sb.st_size != -1) {
+ } else if ((cmd_opts & FL_r) && sb.st_size != -1) {
/* resume mode, local file exists */
- if (!F_flag && us.mtime && sb.st_mtime != us.mtime) {
+ if (!(cmd_opts & FL_F) && us.mtime && sb.st_mtime != us.mtime) {
/* no match! have to refetch */
fclose(f);
/* if precious, warn the user and give up */
- if (R_flag) {
+ if (cmd_opts & FL_R) {
warnx("%s: local modification time "
"does not match remote", path);
goto failure_keep;
@@ -443,7 +447,7 @@
sb = nsb;
}
}
- } else if (m_flag && sb.st_size != -1) {
+ } else if ((cmd_opts & FL_m) && sb.st_size != -1) {
/* mirror mode, local file exists */
if (sb.st_size == us.size && sb.st_mtime == us.mtime)
goto success;
@@ -535,13 +539,16 @@
}
signal(SIGINFO, SIG_DFL);
+ /* free transfer buffer */
+ free(buf);
+
if (timeout)
alarm(0);
stat_end(&xs);
/* set mtime of local file */
- if (!n_flag && us.mtime && !o_stdout
+ if (!(cmd_opts & FL_n) && us.mtime && !o_stdout
&& (stat(path, &sb) != -1) && sb.st_mode & S_IFREG) {
struct timeval tv[2];
@@ -596,10 +603,10 @@
}
goto done;
failure:
- if (of && of != stdout && !R_flag && !r_flag)
+ if (of && of != stdout && !(cmd_opts & FL_R) && !(cmd_opts & FL_r))
if (stat(path, &sb) != -1 && (sb.st_mode & S_IFREG))
unlink(tmppath ? tmppath : path);
- if (R_flag && tmppath != NULL && sb.st_size == -1)
+ if ((cmd_opts & FL_R) && tmppath != NULL && sb.st_size == -1)
rename(tmppath, path); /* ignore errors here */
failure_keep:
r = -1;
@@ -658,7 +665,7 @@
"146AaB:bc:dFf:Hh:lMmnPpo:qRrS:sT:tUvw:")) != EOF)
switch (c) {
case '1':
- once_flag = 1;
+ cmd_opts |= FL_1;
break;
case '4':
family = PF_INET;
@@ -667,10 +674,10 @@
family = PF_INET6;
break;
case 'A':
- A_flag = 1;
+ cmd_opts |= FL_A;
break;
case 'a':
- a_flag = 1;
+ cmd_opts |= FL_a;
break;
case 'B':
if (parseoff(optarg, &B_size) == -1)
@@ -678,16 +685,16 @@
break;
case 'b':
warnx("warning: the -b option is deprecated");
- b_flag = 1;
+ cmd_opts |= FL_b;
break;
case 'c':
c_dirname = optarg;
break;
case 'd':
- d_flag = 1;
+ cmd_opts |= FL_d;
break;
case 'F':
- F_flag = 1;
+ cmd_opts |= FL_F;
break;
case 'f':
f_filename = optarg;
@@ -700,61 +707,61 @@
h_hostname = optarg;
break;
case 'l':
- l_flag = 1;
+ cmd_opts |= FL_l;
break;
case 'o':
- o_flag = 1;
+ cmd_opts |= FL_o;
o_filename = optarg;
break;
case 'M':
case 'm':
- if (r_flag)
+ if (cmd_opts & FL_r)
errx(1, "the -m and -r flags "
"are mutually exclusive");
- m_flag = 1;
+ cmd_opts |= FL_m;
break;
case 'n':
- n_flag = 1;
+ cmd_opts |= FL_n;
break;
case 'P':
case 'p':
- p_flag = 1;
+ cmd_opts |= FL_p;
break;
case 'q':
v_level = 0;
break;
case 'R':
- R_flag = 1;
+ cmd_opts |= FL_R;
break;
case 'r':
- if (m_flag)
+ if (cmd_opts & FL_m)
errx(1, "the -m and -r flags "
"are mutually exclusive");
- r_flag = 1;
+ cmd_opts |= FL_r;
break;
case 'S':
if (parseoff(optarg, &S_size) == -1)
errx(1, "invalid size (%s)", optarg);
break;
case 's':
- s_flag = 1;
+ cmd_opts |= FL_s;
break;
case 'T':
if (parseint(optarg, &T_secs) == -1)
errx(1, "invalid timeout (%s)", optarg);
break;
case 't':
- t_flag = 1;
+ cmd_opts |= FL_t;
warnx("warning: the -t option is deprecated");
break;
case 'U':
- U_flag = 1;
+ cmd_opts |= FL_U;
break;
case 'v':
v_level++;
break;
case 'w':
- a_flag = 1;
+ cmd_opts |= FL_a;
if (parseint(optarg, &w_secs) == -1)
errx(1, "invalid delay (%s)", optarg);
break;
@@ -788,6 +795,7 @@
/* allocate buffer */
if (B_size < MINBUFSIZE)
B_size = MINBUFSIZE;
+
if ((buf = malloc(B_size)) == NULL)
errx(1, "%s", strerror(ENOMEM));
@@ -817,7 +825,7 @@
fetchRestartCalls = 0;
/* output file */
- if (o_flag) {
+ if (cmd_opts & FL_o) {
if (strcmp(o_filename, "-") == 0) {
o_stdout = 1;
} else if (stat(o_filename, &sb) == -1) {
@@ -856,7 +864,7 @@
fetchLastErrCode = 0;
- if (o_flag) {
+ if (cmd_opts & FL_o) {
if (o_stdout) {
e = fetch(*argv, "-");
} else if (o_directory) {
@@ -873,8 +881,8 @@
if (sigint)
kill(getpid(), SIGINT);
- if (e == 0 && once_flag)
- exit(0);
+ if (e == 0 && (cmd_opts & FL_1))
+ exit(EX_OK);
if (e) {
r = 1;
@@ -889,7 +897,7 @@
"before retrying\n", w_secs);
if (w_secs)
sleep(w_secs);
- if (a_flag)
+ if (cmd_opts & FL_a)
continue;
}
}
>Release-Note:
>Audit-Trail:
>Unformatted:
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200208300233.g7U2XB7F023811>