From: Bart Schaefer <schaefer@brasslantern.com>
To: zsh-workers@zsh.org
Subject: PATCH (differently) Re: PATCH: Plug some fd leaks in bin_print
Date: Tue, 06 Jan 2015 23:08:07 -0800 [thread overview]
Message-ID: <150106230807.ZM13656@torch.brasslantern.com> (raw)
In-Reply-To: <150106223532.ZM1050@torch.brasslantern.com>
On Jan 6, 10:35pm, Bart Schaefer wrote:
}
} I have some doubts about some of this. In fact I suspect that these are
} mostly false-positives because the option letters tested in the if()
} conditions preceding each of these goto's are (or should be) mutually
} exclusive.
}
} [...]
}
} This needs to be looked at more closely.
I think this is at least mostly the right thing. There are still some
fishy bits at the very end of the function (where fout is a temp file
or memory map for printf-ing with a format string) that seem to be cases
where a write error could be incorrectly reported, but that's a separate
problem from descriptor leaks.
Does coverity still find leaks after this? What I've done is to move
option validity checking earlier in the function so that fout should
never get opened in the first place if we're not going to make proper
use of it later.
diff --git a/Src/builtin.c b/Src/builtin.c
index 5138c70..f32c29e 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3724,7 +3724,7 @@ int
bin_print(char *name, char **args, Options ops, int func)
{
int flen, width, prec, type, argc, n, narg, curlen = 0;
- int nnl = 0, fmttrunc = 0, ret = 0, maxarg = 0;
+ int nnl = 0, fmttrunc = 0, ret = 0, maxarg = 0, nc = 0;
int flags[5], *len;
char *start, *endptr, *c, *d, *flag, *buf, spec[13], *fmt = NULL;
char **first, **argp, *curarg, *flagch = "0+- #", save = '\0', nullstr = '\0';
@@ -3837,8 +3837,41 @@ bin_print(char *name, char **args, Options ops, int func)
}
}
+ /* -o and -O -- sort the arguments */
+ if (OPT_ISSET(ops,'o') || OPT_ISSET(ops,'O')) {
+ int flags;
+
+ if (fmt && !*args) {
+ if (fout != stdout)
+ fclose(fout);
+ return 0;
+ }
+ flags = OPT_ISSET(ops,'i') ? SORTIT_IGNORING_CASE : 0;
+ if (OPT_ISSET(ops,'O'))
+ flags |= SORTIT_BACKWARDS;
+ strmetasort(args, flags, len);
+ }
+
+ /* -C -- number of columns */
+ if (!fmt && OPT_ISSET(ops,'C')) {
+ char *eptr, *argptr = OPT_ARG(ops,'C');
+ nc = (int)zstrtol(argptr, &eptr, 10);
+ if (*eptr) {
+ zwarnnam(name, "number expcted after -%c: %s", 'C', argptr);
+ return 1;
+ }
+ if (nc <= 0) {
+ zwarnnam(name, "invalid number of columns: %s", argptr);
+ return 1;
+ }
+ }
+
/* -u and -p -- output to other than standard output */
- if (OPT_HASARG(ops,'u') || OPT_ISSET(ops,'p')) {
+ if ((OPT_HASARG(ops,'u') || OPT_ISSET(ops,'p')) &&
+ /* rule out conflicting options -- historical precedence */
+ ((!fmt && (OPT_ISSET(ops,'c') || OPT_ISSET(ops,'C'))) ||
+ !(OPT_ISSET(ops, 'z') ||
+ OPT_ISSET(ops, 's') || OPT_ISSET(ops, 'S')))) {
int fdarg, fd;
if (OPT_ISSET(ops, 'p')) {
@@ -3877,24 +3910,9 @@ bin_print(char *name, char **args, Options ops, int func)
}
}
- /* -o and -O -- sort the arguments */
- if (OPT_ISSET(ops,'o') || OPT_ISSET(ops,'O')) {
- int flags;
-
- if (fmt && !*args) {
- if (fout != stdout)
- fclose(fout);
- return 0;
- }
- flags = OPT_ISSET(ops,'i') ? SORTIT_IGNORING_CASE : 0;
- if (OPT_ISSET(ops,'O'))
- flags |= SORTIT_BACKWARDS;
- strmetasort(args, flags, len);
- }
-
/* -c -- output in columns */
if (!fmt && (OPT_ISSET(ops,'c') || OPT_ISSET(ops,'C'))) {
- int l, nc, nr, sc, n, t, i;
+ int l, nr, sc, n, t, i;
#ifdef MULTIBYTE_SUPPORT
int *widths;
@@ -3965,19 +3983,9 @@ bin_print(char *name, char **args, Options ops, int func)
#endif
if (OPT_ISSET(ops,'C')) {
- char *eptr, *argptr = OPT_ARG(ops,'C');
- nc = (int)zstrtol(argptr, &eptr, 10);
- if (*eptr) {
- zwarnnam(name, "number expcted after -%c: %s", 'C', argptr);
- return 1;
- }
- if (nc <= 0) {
- zwarnnam(name, "invalid number of columns: %s", argptr);
- return 1;
- }
/*
* n: number of elements
- * nc: number of columns
+ * nc: number of columns (above)
* nr: number of rows
*/
n = arrlen(args);
next prev parent reply other threads:[~2015-01-07 7:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-07 0:25 Mikael Magnusson
2015-01-07 1:21 ` Bart Schaefer
2015-01-07 3:15 ` Mikael Magnusson
2015-01-07 6:35 ` Bart Schaefer
2015-01-07 7:08 ` Bart Schaefer [this message]
2015-01-07 7:14 ` PATCH (differently) " Bart Schaefer
2015-01-07 8:26 ` Mikael Magnusson
2015-01-07 17:04 ` Bart Schaefer
2015-01-07 8:04 ` Mikael Magnusson
2015-01-07 8:06 ` Mikael Magnusson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=150106230807.ZM13656@torch.brasslantern.com \
--to=schaefer@brasslantern.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).