[hackers] [sbase] untypedef expr, find, test, as is existing style in sbase || Evan Gates
commit cab14bf253767fa9f469eef5808f95045ce85ab0
Author: Evan Gates
Date: Tue Mar 17 10:59:16 2015 -0700
untypedef expr, find, test, as is existing style in sbase
diff --git a/expr.c b/expr.c
index aa2bea8..ef7c303 100644
--- a/expr.c
+++ b/expr.c
@@ -12,15 +12,15 @@ enum {
VAL = CHAR_MAX + 1, GE, LE, NE
};
-typedef struct {
- char *s; /* iff s is NULL, Val is an integer */
+struct val {
+ char *s; /* iff s is NULL, val is an integer */
intmax_t n;
-} Val;
+};
static size_t intlen;
static void
-enan(Val v)
+enan(struct val v)
{
if (v.s)
enprintf(2, "syntax error: expected integer got `%s'\n", v.s);
@@ -34,7 +34,7 @@ ezero(intmax_t n)
}
static char *
-valstr(Val val, char *buf, size_t bufsiz)
+valstr(struct val val, char *buf, size_t bufsiz)
{
if (val.s)
return val.s;
@@ -43,7 +43,7 @@ valstr(Val val, char *buf, size_t bufsiz)
}
static int
-valcmp(Val a, Val b)
+valcmp(struct val a, struct val b)
{
char buf1[intlen], buf2[intlen];
char *astr = valstr(a, buf1, sizeof(buf1));
@@ -59,8 +59,8 @@ valcmp(Val a, Val b)
* then return the text matched by it \1 (empty string for no match)
* else return number of characters matched (0 for no match)
*/
-static Val
-match(Val vstr, Val vregx)
+static struct val
+match(struct val vstr, struct val vregx)
{
regex_t re;
regmatch_t matches[2];
@@ -76,7 +76,7 @@ match(Val vstr, Val vregx)
if (regexec(&re, str, 2, matches, 0)) {
regfree(&re);
- return (Val){ (re.re_nsub ? "" : NULL), 0 };
+ return (struct val){ (re.re_nsub ? "" : NULL), 0 };
}
if (re.re_nsub) {
@@ -87,15 +87,15 @@ match(Val vstr, Val vregx)
*p = '\0';
d = strtoimax(s, &p, 10);
if (*s && !*p) /* string matched by subexpression is an integer
*/
- return (Val){ NULL, d };
+ return (struct val){ NULL, d };
/* FIXME? string is never free()d, worth fixing?
* need to allocate as it could be in buf1 instead of vstr.s */
- return (Val){ enstrdup(3, s), 0 };
+ return (struct val){ enstrdup(3, s), 0 };
}
regfree(&re);
str += matches[0].rm_so;
- return (Val){ NULL, utfnlen(str, matches[0].rm_eo - matches[0].rm_so) };
+ return (struct val){ NULL, utfnlen(str, matches[0].rm_eo -
matches[0].rm_so) };
}
/* ops points to a stack of operators, opp points to one past the last op
@@ -105,9 +105,9 @@ match(Val vstr, Val vregx)
* pop operator, pop two values, apply operator, push result
*/
static void
-doop(int *ops, int **opp, Val *vals, Val **valp)
+doop(int *ops, int **opp, struct val *vals, struct val **valp)
{
- Val ret, a, b;
+ struct val ret, a, b;
int op;
/* For an operation, we need a valid operator
@@ -123,30 +123,30 @@ doop(int *ops, int **opp, Val *vals, Val **valp)
switch (op) {
case '|':
- if ( a.s && *a.s) ret = (Val){ a.s , 0 };
- else if (!a.s && a.n) ret = (Val){ NULL, a.n };
- else if ( b.s && *b.s) ret = (Val){ b.s , 0 };
- else ret = (Val){ NULL, b.n };
+ if ( a.s && *a.s) ret = (struct val){ a.s , 0 };
+ else if (!a.s && a.n) ret = (struct val){ NULL, a.n };
+ else if ( b.s && *b.s) ret = (struct val){ b.s , 0 };
+ else ret = (struct val){ NULL, b.n };
break;
case '&':
if (((a.s && *a.s) || a.n) && ((b.s && *b.s) || b.n))
ret = a;
else
- ret = (Val){ NULL, 0 };
+ ret = (struct val){ NULL, 0 };
break;
- case '=': ret = (Val){ NULL, valcmp(a, b) == 0 }; break;
- case '>': ret = (Val){ NULL, valcmp(a, b) > 0 }; break;
- case GE : ret = (Val){ NULL, valcmp(a, b) >= 0 }; break;
- case '<': ret = (Val){ NULL, valcmp(a, b) < 0 }; break;
- case LE : ret = (Val){ NULL, valcmp(a, b) <= 0 }; break;
- case NE : ret = (Val){ NULL, valcmp(a, b) != 0 }; break;
+ case '=': ret = (struct val){ NULL, valcmp(a, b) == 0 }; break;
+ case '>': ret = (struct val){ NULL, valcmp(a, b) > 0 }; break;
+ case GE : ret = (struct val){ NULL, valcmp(a, b) >= 0 }; break;
+ case '<': ret = (struct val){ NULL, valcmp(a, b) < 0 }; break;
+ case LE : ret = (struct val){ NULL, valcmp(a, b) <= 0 }; break;
+ case NE : ret = (struct val){ NULL, valcmp(a, b) != 0 }; break;
- case '+': enan(a); enan(b); ret = (Val){ NULL, a.n + b.n };
break;
- case '-': enan(a); enan(b); ret = (Val){ NULL, a.n - b.n };
break;
- case '*': enan(a); enan(b); ret = (Val){ NULL, a.n
Re: [hackers] [sbase] Rewrite foldline() in fold(1) || FRIGN
> If fwrite() fails with a short element count, will ferror() always be > on true? I am not sure we can rely on this. > > And also if fwrite() fails is it guaranteed that it will continue to > fail on all future calls? Yes, it is. If an error is produced in a stream, the value of error associated with the strem is kept until is cleared with a clearerr call. Regards,
Re: [hackers] [sbase] Rewrite foldline() in fold(1) || FRIGN
On Tue, Mar 17, 2015 at 11:35:33AM +0100, Roberto E. Vargas Caballero wrote:
>
> Hi,
>
> > + for (p = str, col = 0; *p && *p != '\n'; p++) {
> > + if (!UTF8_POINT(*p) && !bflag)
> > + continue;
> > + if (col >= width) {
> > + off = (sflag && spacesect) ? spacesect - str : p - str;
> > + if (fwrite(str, 1, off, stdout) != off)
> > + eprintf("fwrite :");
> > + putchar('\n');
> ...
> > + fputs(str, stdout);
>
> It's a bit strange this fwrite, why don't you use putchar there?, and
> why you check the return value of fwrite, but not the return value of
> putcharor fputs?. I usually don't check the return value of write
> functions and at the end of the loop I do a call to ferror.
If fwrite() fails with a short element count, will ferror() always be
on true? I am not sure we can rely on this.
And also if fwrite() fails is it guaranteed that it will continue to
fail on all future calls?
Re: [hackers] [sbase] Rewrite foldline() in fold(1) || FRIGN
On Tue, 17 Mar 2015 11:35:33 +0100 "Roberto E. Vargas Caballero" wrote: Hey Roberto, > It's a bit strange this fwrite, why don't you use putchar there? Look again, we write 'off' bytes, not 1. > and why you check the return value of fwrite, but not the return value of > putchar or fputs?. I usually don't check the return value of write > functions and at the end of the loop I do a call to ferror. Calling ferror sounds like a nice idea. Let's take a look at it. Cheers FRIGN -- FRIGN
Re: [hackers] [sbase] Rewrite foldline() in fold(1) || FRIGN
Hi,
> + for (p = str, col = 0; *p && *p != '\n'; p++) {
> + if (!UTF8_POINT(*p) && !bflag)
> + continue;
> + if (col >= width) {
> + off = (sflag && spacesect) ? spacesect - str : p - str;
> + if (fwrite(str, 1, off, stdout) != off)
> + eprintf("fwrite :");
> + putchar('\n');
...
> + fputs(str, stdout);
It's a bit strange this fwrite, why don't you use putchar there?, and
why you check the return value of fwrite, but not the return value of
putcharor fputs?. I usually don't check the return value of write
functions and at the end of the loop I do a call to ferror.
Regards,
[hackers] [sbase] Add estrlcat() and estrlcpy() || FRIGN
commit 8729b6ba39a21229cf7de8ebc40960079c4bb1bf
Author: FRIGN
Date: Tue Mar 17 11:24:49 2015 +0100
Add estrlcat() and estrlcpy()
It has become a common idiom in sbase to check strlcat() and strlcpy()
using
if (strl{cat, cpy}(dst, src, siz) >= siz)
eprintf("path too long\n");
However, this was not carried out consistently and to this very day,
some tools employed unchecked calls to these functions, effectively
allowing silent truncations to happen, which in turn may lead to
security issues.
To finally put an end to this, the e*-functions detect truncation
automatically and the caller can lean back and enjoy coding without
trouble. :)
diff --git a/find.c b/find.c
index 9b876dc..9a427a2 100644
--- a/find.c
+++ b/find.c
@@ -977,10 +977,10 @@ find(char *path, Findhist *hist)
if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
continue;
- p = pathbuf + strlcpy(pathbuf, path, pathcap);
+ p = pathbuf + estrlcpy(pathbuf, path, pathcap);
if (*--p != '/')
- strlcat(pathbuf, "/", pathcap);
- strlcat(pathbuf, de->d_name, pathcap);
+ estrlcat(pathbuf, "/", pathcap);
+ estrlcat(pathbuf, de->d_name, pathcap);
find(pathbuf, &cur);
}
closedir(dir); /* check return value? */
diff --git a/libutil/recurse.c b/libutil/recurse.c
index 1637d0c..c4e8043 100644
--- a/libutil/recurse.c
+++ b/libutil/recurse.c
@@ -66,12 +66,10 @@ recurse(const char *path, void *data, struct recursor *r)
}
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue;
- if (strlcpy(subpath, path, PATH_MAX) >= PATH_MAX)
- eprintf("strlcpy: path too long\n");
- if (path[strlen(path) - 1] != '/' && strlcat(subpath, "/",
PATH_MAX) >= PATH_MAX)
- eprintf("strlcat: path too long\n");
- if (strlcat(subpath, d->d_name, PATH_MAX) >= PATH_MAX)
- eprintf("strlcat: path too long\n");
+ estrlcpy(subpath, path, sizeof(subpath));
+ if (path[strlen(path) - 1] != '/')
+ estrlcat(subpath, "/", sizeof(subpath));
+ estrlcat(subpath, d->d_name, sizeof(subpath));
if ((r->flags & SAMEDEV) && statf(subpath, &dst) < 0) {
if (errno != ENOENT) {
weprintf("%s %s:", statf_name, subpath);
diff --git a/libutil/strlcat.c b/libutil/strlcat.c
index 9e2d251..bf263b8 100644
--- a/libutil/strlcat.c
+++ b/libutil/strlcat.c
@@ -50,3 +50,14 @@ strlcat(char *dst, const char *src, size_t siz)
*d = '\0';
return(dlen + (s - src)); /* count does not include NUL */
}
+
+size_t
+estrlcat(char *dst, const char *src, size_t siz)
+{
+ size_t ret;
+
+ if ((ret = strlcat(dst, src, siz)) >= siz)
+ eprintf("strlcat: input string too long\n");
+
+ return ret;
+}
diff --git a/libutil/strlcpy.c b/libutil/strlcpy.c
index 388b426..44b618a 100644
--- a/libutil/strlcpy.c
+++ b/libutil/strlcpy.c
@@ -46,3 +46,14 @@ strlcpy(char *dst, const char *src, size_t siz)
}
return(s - src - 1); /* count does not include NUL */
}
+
+size_t
+estrlcpy(char *dst, const char *src, size_t siz)
+{
+ size_t ret;
+
+ if ((ret = strlcpy(dst, src, siz)) >= siz)
+ eprintf("strlcpy: input string too long\n");
+
+ return ret;
+}
diff --git a/logger.c b/logger.c
index 0f8a763..80dcdc7 100644
--- a/logger.c
+++ b/logger.c
@@ -80,9 +80,9 @@ main(int argc, char *argv[])
sz += argc;
buf = ecalloc(1, sz);
for (i = 0; i < argc; i++) {
- strlcat(buf, argv[i], sz);
+ estrlcat(buf, argv[i], sz);
if (i + 1 < argc)
- strlcat(buf, " ", sz);
+ estrlcat(buf, " ", sz);
}
syslog(priority, "%s", buf);
}
diff --git a/mktemp.c b/mktemp.c
index 8dbb210..b4ecaf1 100644
--- a/mktemp.c
+++ b/mktemp.c
@@ -39,19 +39,14 @@ main(int argc, char *argv[])
if ((p = getenv("TMPDIR")))
tmpdir = p;
- if (strlcpy(tmp, template, sizeof(tmp)) >= sizeof(tmp))
- eprintf("path too long\n");
+ estrlcpy(tmp, template, sizeof(tmp));
p = dirname(tmp);
if (p[0] != '.') {
- if (strlcpy(path, template, sizeof(path)) >= sizeof(path))
- eprintf("path too long\n");
+ estrlcpy(path, template, sizeof(path));
} else {
- if (strlcpy(path, tmpdir, sizeof(path)) >= sizeof(path))
- eprintf("path too long\n");
- if (strlcat(path, "/", sizeof(path)
[hackers] [sbase] Rewrite foldline() in fold(1) || FRIGN
commit 9d151741258d6e2f7eb415b8783a2c2a9efbda04
Author: FRIGN
Date: Mon Mar 16 19:26:42 2015 +0100
Rewrite foldline() in fold(1)
After the audit, I had this noted down as a TODO-item, but
considered the function to be tested enough to hold the line
until I came to rewrite it.
Admittedly, I didn't take a closer look at the previous loop
and there probably were some edge-cases which caused trouble, but
so far so good, the new version of this commit should be safe
and considered audited.
diff --git a/fold.c b/fold.c
index 020d429..c176497 100644
--- a/fold.c
+++ b/fold.c
@@ -3,6 +3,7 @@
#include
#include
#include
+#include
#include "util.h"
@@ -11,46 +12,41 @@ static intsflag = 0;
static size_t width = 80;
static void
-foldline(const char *str)
-{
- size_t i = 0, n = 0, col, j;
- int space;
- char c;
-
- do {
- space = 0;
- for (j = i, col = 0; str[j] && col <= width; j++) {
- c = str[j];
- if (!UTF8_POINT(c) && !bflag)
- continue;
- if (sflag && isspace(c)) {
- space = 1;
- n = j + 1;
- } else if (!space) {
- n = j;
- }
+foldline(const char *str) {
+ const char *p, *spacesect = NULL;
+ size_t col, off;
- if (!bflag && iscntrl(c)) {
- switch(c) {
- case '\b':
- col--;
- break;
- case '\r':
- col = 0;
- break;
- case '\t':
- col += (col + 1) % 8;
- break;
- }
- } else {
- col++;
+ for (p = str, col = 0; *p && *p != '\n'; p++) {
+ if (!UTF8_POINT(*p) && !bflag)
+ continue;
+ if (col >= width) {
+ off = (sflag && spacesect) ? spacesect - str : p - str;
+ if (fwrite(str, 1, off, stdout) != off)
+ eprintf("fwrite :");
+ putchar('\n');
+ spacesect = NULL;
+ col = 0;
+ p = str += off;
+ }
+ if (sflag && isspace(*p))
+ spacesect = p + 1;
+ if (!bflag && iscntrl(*p)) {
+ switch(*p) {
+ case '\b':
+ col -= (col > 0);
+ break;
+ case '\r':
+ col = 0;
+ break;
+ case '\t':
+ col += (col + 1) % 8;
+ break;
}
+ } else {
+ col++;
}
- if (fwrite(str + i, 1, n - i, stdout) != n - i)
- eprintf("fwrite :");
- if (str[n])
- putchar('\n');
- } while (str[i = n] && str[i] != '\n');
+ }
+ fputs(str, stdout);
}
static void
@@ -69,7 +65,7 @@ fold(FILE *fp, const char *fname)
static void
usage(void)
{
- eprintf("usage: %s [-bs] [-w width | -width] [FILE...]\n", argv0);
+ eprintf("usage: %s [-bs] [-w num | -num] [FILE ...]\n", argv0);
}
int
@@ -102,10 +98,10 @@ main(int argc, char *argv[])
if (!(fp = fopen(*argv, "r"))) {
weprintf("fopen %s:", *argv);
ret = 1;
- continue;
+ } else {
+ fold(fp, *argv);
+ fclose(fp);
}
- fold(fp, *argv);
- fclose(fp);
}
}
[hackers] [sbase] Audit logname(1) || FRIGN
commit 4d25d87ada83b40620579c2104386f246568cb10
Author: FRIGN
Date: Tue Mar 17 00:44:18 2015 +0100
Audit logname(1)
1) Add usage().
2) Idiomatic argv0-setter. We don't use arg.h, as we do not process
flags or arguments.
3) Remove program-name from eprintf-call. This is done in the eprintf-
function itself when the DEBUG-define is set.
We'll activate it by default later.
4) Add empty line before return.
diff --git a/README b/README
index ecace49..c936841 100644
--- a/README
+++ b/README
@@ -41,7 +41,7 @@ The following tools are implemented ('*' == finished, '#' ==
UTF-8 support,
=*| linkyes none
=*| ln yes none
=*| logger yes none
-=* logname yes none
+=*| logname yes none
= ls no (-C), -S, -f, -m, -s, -x
=*| md5sum non-posixnone
=*| mkdir yes none
diff --git a/logname.c b/logname.c
index f1981a5..11d287b 100644
--- a/logname.c
+++ b/logname.c
@@ -4,17 +4,26 @@
#include "util.h"
+static void
+usage(void)
+{
+ eprintf("usage: %s\n", argv0);
+}
+
int
main(int argc, char *argv[])
{
char *login;
- argv0 = argv[0];
- if (argc != 1)
- eprintf("usage: %s\n", argv0);
+ argv0 = argv[0], argc--, argv++;
+
+ if (argc)
+ usage();
+
if ((login = getlogin()))
puts(login);
else
- eprintf("%s: no login name\n", argv0);
+ eprintf("no login name\n");
+
return 0;
}
[hackers] [sbase] Audit mktemp(1) || FRIGN
commit f724e87cc51c58404983439650f03851c897172f
Author: FRIGN
Date: Tue Mar 17 11:01:33 2015 +0100
Audit mktemp(1)
1) Unglobalize variables.
2) Sort local variables.
3) Use return instead of exit() in main().
4) Add empty line before return.
diff --git a/README b/README
index c936841..7c29823 100644
--- a/README
+++ b/README
@@ -46,7 +46,7 @@ The following tools are implemented ('*' == finished, '#' ==
UTF-8 support,
=*| md5sum non-posixnone
=*| mkdir yes none
=*| mkfifo yes none
-=* mktemp non-posixnone
+=*| mktemp non-posixnone
=*| mv yes none (-i)
=*| niceyes none
= nl no -d, -f, -h, -p
diff --git a/mktemp.c b/mktemp.c
index 08796cd..8dbb210 100644
--- a/mktemp.c
+++ b/mktemp.c
@@ -12,16 +12,13 @@ usage(void)
eprintf("usage: %s [-dq] [template]\n", argv0);
}
-static int dflag = 0;
-static int qflag = 0;
-
int
main(int argc, char *argv[])
{
- char *template = "tmp.XX";
- char *tmpdir = "/tmp", *p;
- char path[PATH_MAX], tmp[PATH_MAX];
- int fd;
+ int dflag = 0, qflag = 0, fd;
+ char *template = "tmp.XX",
+*tmpdir = "/tmp", *p,
+ path[PATH_MAX], tmp[PATH_MAX];
ARGBEGIN {
case 'd':
@@ -61,16 +58,17 @@ main(int argc, char *argv[])
if (!mkdtemp(path)) {
if (!qflag)
eprintf("mkdtemp %s:", path);
- exit(1);
+ return 1;
}
} else {
if ((fd = mkstemp(path)) < 0) {
if (!qflag)
eprintf("mkstemp %s:", path);
- exit(1);
+ return 1;
}
close(fd);
}
puts(path);
+
return 0;
}
