From owner-freebsd-bugs Thu Aug 29 19:40:44 2002 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 43ADA37B401 for ; Thu, 29 Aug 2002 19:40:09 -0700 (PDT) Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7D54D43E6A for ; Thu, 29 Aug 2002 19:40:07 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.4/8.12.4) with ESMTP id g7U2e7JU027831 for ; Thu, 29 Aug 2002 19:40:07 -0700 (PDT) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.4/8.12.4/Submit) id g7U2e7hX027830; Thu, 29 Aug 2002 19:40:07 -0700 (PDT) Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 97AC137B400 for ; Thu, 29 Aug 2002 19:33:24 -0700 (PDT) Received: from xor.sqrt.ca (h24-82-193-97.wp.shawcable.net [24.82.193.97]) by mx1.FreeBSD.org (Postfix) with ESMTP id 126FB43E88 for ; Thu, 29 Aug 2002 19:33:13 -0700 (PDT) (envelope-from modulus@xor.sqrt.ca) Received: from xor.sqrt.ca (smmsp@localhost [127.0.0.1]) by xor.sqrt.ca (8.12.5/8.12.5) with ESMTP id g7U2XBW2023813 for ; Thu, 29 Aug 2002 21:33:11 -0500 (CDT) (envelope-from modulus@xor.sqrt.ca) Received: (from root@localhost) by xor.sqrt.ca (8.12.5/8.12.5/Submit) id g7U2XB7F023811; Thu, 29 Aug 2002 21:33:11 -0500 (CDT) Message-Id: <200208300233.g7U2XB7F023811@xor.sqrt.ca> Date: Thu, 29 Aug 2002 21:33:11 -0500 (CDT) From: "Chris S.J. Peron" Reply-To: "Chris S.J. Peron" To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: bin/42179: [patch] fetch: memory leak and general optimizations Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org >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