Discussion:
[sbase] [PATCH v2 00/11] IO improvements and some bug fixes
(too old to reply)
Michael Forney
2016-12-06 10:16:52 UTC
Permalink
Thanks to Silvan, Hiltjo, and Laslo for their comments.

Changes since v1:
- Changed concat to return -2 on write error so that callers have the option to
handle it differently.
- Added a patch to libutil/cp.c to preserve atime/mtime for symlinks.
- Changed the od overflow fix to be a bit clearer and avoid casting off_t to
size_t.
- Switched tail to use raw IO and concat as well.
- Added a patch to tail to make -c offsets refer to bytes instead of runes.

Michael Forney (11):
crypt: Add some missing error checks for cryptsum
od: Fix buffer overflow if -N flag is larger than BUFSIZ
libutil: Add writeall utility function
Don't use buffered IO (fread) when not appropriate
xinstall: Check result of fchmod
concat: Use plain read/write instead of buffered stdio
cp: Only call chmod with -p or -a
tail: Use fstat in case file is removed
cp: Check result of utimensat
cp: Also preserve atime/mtime for symlinks
tail: Process bytes with -c option, and add -m option for runes

Makefile | 3 +-
cat.c | 39 ++++-----
cksum.c | 31 +++----
crypt.h | 2 +-
libutil/concat.c | 24 +++---
libutil/cp.c | 56 ++++++-------
libutil/crypt.c | 47 ++++++-----
libutil/writeall.c | 21 +++++
od.c | 55 ++++++++-----
sponge.c | 31 +++----
tail.1 | 6 +-
tail.c | 236 +++++++++++++++++++++++++++++++++--------------------
tee.c | 39 +++++----
text.h | 1 -
util.h | 4 +
xinstall.c | 26 +++---
16 files changed, 358 insertions(+), 263 deletions(-)
create mode 100644 libutil/writeall.c
--
2.10.2
Michael Forney
2016-12-06 10:16:53 UTC
Permalink
Previously, if a file failed to read in a checksum list, it would be
reported as not matched rather than a read failure.

Also, if reading from stdin failed, previously a bogus checksum would be
printed anyway.
---
libutil/crypt.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/libutil/crypt.c b/libutil/crypt.c
index 3f849ba..6991c39 100644
--- a/libutil/crypt.c
+++ b/libutil/crypt.c
@@ -64,7 +64,10 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
(*noread)++;
continue;
}
- cryptsum(ops, fp, file, md);
+ if (cryptsum(ops, fp, file, md)) {
+ (*noread)++;
+ continue;
+ }
r = mdcheckline(line, md, sz);
if (r == 1) {
printf("%s: OK\n", file);
@@ -125,8 +128,10 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
int ret = 0;

if (argc == 0) {
- cryptsum(ops, stdin, "<stdin>", md);
- mdprint(md, "<stdin>", sz);
+ if (cryptsum(ops, stdin, "<stdin>", md))
+ ret = 1;
+ else
+ mdprint(md, "<stdin>", sz);
} else {
for (; *argv; argc--, argv++) {
if ((*argv)[0] == '-' && !(*argv)[1]) {
@@ -137,11 +142,10 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
ret = 1;
continue;
}
- if (cryptsum(ops, fp, *argv, md)) {
+ if (cryptsum(ops, fp, *argv, md))
ret = 1;
- } else {
+ else
mdprint(md, *argv, sz);
- }
if (fp != stdin && fshut(fp, *argv))
ret = 1;
}
--
2.10.2
Laslo Hunhold
2016-12-27 13:32:50 UTC
Permalink
On Tue, 6 Dec 2016 02:16:53 -0800
Michael Forney <***@mforney.org> wrote:

Hey Michael,
Post by Michael Forney
Previously, if a file failed to read in a checksum list, it would be
reported as not matched rather than a read failure.
Also, if reading from stdin failed, previously a bogus checksum would
be printed anyway.
Thanks, applied! I added a note on mdcheckline() in TODO so we don't
miss it.

Cheers

Laslo
--
Laslo Hunhold <***@frign.de>
Michael Forney
2016-12-06 10:16:54 UTC
Permalink
Previously, if max was specified, od will call read with that size,
potentially overflowing buf with data read from the file.
---
od.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/od.c b/od.c
index 9b83501..27a7104 100644
--- a/od.c
+++ b/od.c
@@ -129,23 +129,25 @@ od(FILE *fp, char *fname, int last)
{
static unsigned char *line;
static size_t lineoff;
- size_t i;
- unsigned char buf[BUFSIZ];
static off_t addr;
- size_t buflen;
+ unsigned char buf[BUFSIZ];
+ size_t i, n, size = sizeof(buf);

while (skip - addr > 0) {
- buflen = fread(buf, 1, MIN(skip - addr, BUFSIZ), fp);
- addr += buflen;
+ n = fread(buf, 1, MIN(skip - addr, sizeof(buf)), fp);
+ addr += n;
if (feof(fp) || ferror(fp))
return;
}
if (!line)
line = emalloc(linelen);

- while ((buflen = fread(buf, 1, max >= 0 ?
- max - (addr - skip) : BUFSIZ, fp))) {
- for (i = 0; i < buflen; i++, addr++) {
+ for (;;) {
+ if (max >= 0)
+ size = MIN(max - (addr - skip), size);
+ if (!(n = fread(buf, 1, size, fp)))
+ break;
+ for (i = 0; i < n; i++, addr++) {
line[lineoff++] = buf[i];
if (lineoff == linelen) {
printline(line, lineoff, addr - lineoff + 1);
--
2.10.2
Laslo Hunhold
2016-12-27 13:33:02 UTC
Permalink
On Tue, 6 Dec 2016 02:16:54 -0800
Post by Michael Forney
Previously, if max was specified, od will call read with that size,
potentially overflowing buf with data read from the file.
Thanks, applied!
--
Laslo Hunhold <***@frign.de>
Michael Forney
2016-12-06 10:16:55 UTC
Permalink
writeall makes successive write calls to write an entire buffer to the
output file descriptor. It returns the number of bytes written, or -1 on
the first error.
---
Makefile | 3 ++-
libutil/writeall.c | 21 +++++++++++++++++++++
util.h | 3 +++
3 files changed, 26 insertions(+), 1 deletion(-)
create mode 100644 libutil/writeall.c

diff --git a/Makefile b/Makefile
index 25bab70..a337ead 100644
--- a/Makefile
+++ b/Makefile
@@ -79,7 +79,8 @@ LIBUTILSRC =\
libutil/strlcpy.c\
libutil/strsep.c\
libutil/strtonum.c\
- libutil/unescape.c
+ libutil/unescape.c\
+ libutil/writeall.c

LIB = $(LIBUTF) $(LIBUTIL)

diff --git a/libutil/writeall.c b/libutil/writeall.c
new file mode 100644
index 0000000..4725ced
--- /dev/null
+++ b/libutil/writeall.c
@@ -0,0 +1,21 @@
+/* See LICENSE file for copyright and license details. */
+#include <unistd.h>
+
+#include "../util.h"
+
+ssize_t
+writeall(int fd, const void *buf, size_t len)
+{
+ const char *p = buf;
+ ssize_t n;
+
+ while (len) {
+ n = write(fd, p, len);
+ if (n <= 0)
+ return n;
+ p += n;
+ len -= n;
+ }
+
+ return p - (const char *)buf;
+}
diff --git a/util.h b/util.h
index b5860dc..eaad3ce 100644
--- a/util.h
+++ b/util.h
@@ -62,6 +62,9 @@ char *strsep(char **, const char *);
int enregcomp(int, regex_t *, const char *, int);
int eregcomp(regex_t *, const char *, int);

+/* io */
+ssize_t writeall(int, const void *, size_t);
+
/* misc */
void enmasse(int, char **, int (*)(const char *, const char *, int));
void fnck(const char *, const char *, int (*)(const char *, const char *, int), int);
--
2.10.2
Michael Forney
2016-12-06 10:16:56 UTC
Permalink
fread reads the entire requested size (BUFSIZ), which causes tools to
block if only small amounts of data are available at a time. At best,
this causes unnecessary copies and inefficiency, at worst, tools like
tee and cat are almost unusable in some cases since they only display
large chunks of data at a time.
---
cksum.c | 31 +++++++++++++++++--------------
crypt.h | 2 +-
libutil/crypt.c | 37 +++++++++++++++++++------------------
od.c | 43 +++++++++++++++++++++++++++----------------
tee.c | 39 +++++++++++++++++++--------------------
5 files changed, 83 insertions(+), 69 deletions(-)

diff --git a/cksum.c b/cksum.c
index 570ca81..b53ec17 100644
--- a/cksum.c
+++ b/cksum.c
@@ -1,7 +1,9 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <inttypes.h>
#include <stdio.h>
#include <string.h>
+#include <unistd.h>

#include "util.h"

@@ -61,19 +63,20 @@ static const unsigned long crctab[] = { 0x00000000,
};

static void
-cksum(FILE *fp, const char *s)
+cksum(int fd, const char *s)
{
- size_t len = 0, i, n;
+ ssize_t n;
+ size_t len = 0, i;
uint32_t ck = 0;
unsigned char buf[BUFSIZ];

- while ((n = fread(buf, 1, sizeof(buf), fp))) {
+ while ((n = read(fd, buf, sizeof(buf))) > 0) {
for (i = 0; i < n; i++)
ck = (ck << 8) ^ crctab[(ck >> 24) ^ buf[i]];
len += n;
}
- if (ferror(fp)) {
- weprintf("fread %s:", s ? s : "<stdin>");
+ if (n < 0) {
+ weprintf("read %s:", s ? s : "<stdin>");
ret = 1;
return;
}
@@ -92,29 +95,29 @@ cksum(FILE *fp, const char *s)
int
main(int argc, char *argv[])
{
- FILE *fp;
+ int fd;

argv0 = argv[0], argc--, argv++;

if (!argc) {
- cksum(stdin, NULL);
+ cksum(0, NULL);
} else {
for (; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- cksum(fp, *argv);
- if (fp != stdin && fshut(fp, *argv))
- ret = 1;
+ cksum(fd, *argv);
+ if (fd != 0)
+ close(fd);
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>");
+ ret |= fshut(stdout, "<stdout>");

return ret;
}
diff --git a/crypt.h b/crypt.h
index e0cc08d..2fd2932 100644
--- a/crypt.h
+++ b/crypt.h
@@ -8,5 +8,5 @@ struct crypt_ops {

int cryptcheck(int, char **, struct crypt_ops *, uint8_t *, size_t);
int cryptmain(int, char **, struct crypt_ops *, uint8_t *, size_t);
-int cryptsum(struct crypt_ops *, FILE *, const char *, uint8_t *);
+int cryptsum(struct crypt_ops *, int, const char *, uint8_t *);
void mdprint(const uint8_t *, const char *, size_t);
diff --git a/libutil/crypt.c b/libutil/crypt.c
index 6991c39..e285614 100644
--- a/libutil/crypt.c
+++ b/libutil/crypt.c
@@ -1,8 +1,10 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>

#include "../crypt.h"
#include "../text.h"
@@ -41,7 +43,7 @@ static void
mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
int *formatsucks, int *noread, int *nonmatch)
{
- FILE *fp;
+ int fd;
size_t bufsiz = 0;
int r;
char *line = NULL, *file, *p;
@@ -59,12 +61,12 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
file += 2;
for (p = file; *p && *p != '\n' && *p != '\r'; p++); /* strip newline */
*p = '\0';
- if (!(fp = fopen(file, "r"))) {
- weprintf("fopen %s:", file);
+ if ((fd = open(file, O_RDONLY)) < 0) {
+ weprintf("open %s:", file);
(*noread)++;
continue;
}
- if (cryptsum(ops, fp, file, md)) {
+ if (cryptsum(ops, fd, file, md)) {
(*noread)++;
continue;
}
@@ -77,7 +79,7 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
} else {
(*formatsucks)++;
}
- fclose(fp);
+ close(fd);
}
free(line);
}
@@ -124,11 +126,11 @@ cryptcheck(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz
int
cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
{
- FILE *fp;
+ int fd;
int ret = 0;

if (argc == 0) {
- if (cryptsum(ops, stdin, "<stdin>", md))
+ if (cryptsum(ops, 0, "<stdin>", md))
ret = 1;
else
mdprint(md, "<stdin>", sz);
@@ -136,18 +138,18 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
for (; *argv; argc--, argv++) {
if ((*argv)[0] == '-' && !(*argv)[1]) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- if (cryptsum(ops, fp, *argv, md))
+ if (cryptsum(ops, fd, *argv, md))
ret = 1;
else
mdprint(md, *argv, sz);
- if (fp != stdin && fshut(fp, *argv))
- ret = 1;
+ if (fd != 0)
+ close(fd);
}
}

@@ -155,16 +157,15 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
}

int
-cryptsum(struct crypt_ops *ops, FILE *fp, const char *f,
- uint8_t *md)
+cryptsum(struct crypt_ops *ops, int fd, const char *f, uint8_t *md)
{
uint8_t buf[BUFSIZ];
- size_t n;
+ ssize_t n;

ops->init(ops->s);
- while ((n = fread(buf, 1, sizeof(buf), fp)) > 0)
+ while ((n = read(fd, buf, sizeof(buf))) > 0)
ops->update(ops->s, buf, n);
- if (ferror(fp)) {
+ if (n < 0) {
weprintf("%s: read error:", f);
return 1;
}
diff --git a/od.c b/od.c
index 27a7104..e5dde83 100644
--- a/od.c
+++ b/od.c
@@ -1,8 +1,10 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>

#include "queue.h"
#include "util.h"
@@ -124,20 +126,23 @@ once:
}
}

-static void
-od(FILE *fp, char *fname, int last)
+static int
+od(int fd, char *fname, int last)
{
static unsigned char *line;
static size_t lineoff;
static off_t addr;
unsigned char buf[BUFSIZ];
- size_t i, n, size = sizeof(buf);
+ size_t i, size = sizeof(buf);
+ ssize_t n;

while (skip - addr > 0) {
- n = fread(buf, 1, MIN(skip - addr, sizeof(buf)), fp);
+ n = read(fd, buf, MIN(skip - addr, sizeof(buf)));
+ if (n < 0)
+ weprintf("read %s:", fname);
+ if (n <= 0)
+ return n;
addr += n;
- if (feof(fp) || ferror(fp))
- return;
}
if (!line)
line = emalloc(linelen);
@@ -145,7 +150,7 @@ od(FILE *fp, char *fname, int last)
for (;;) {
if (max >= 0)
size = MIN(max - (addr - skip), size);
- if (!(n = fread(buf, 1, size, fp)))
+ if ((n = read(fd, buf, size)) <= 0)
break;
for (i = 0; i < n; i++, addr++) {
line[lineoff++] = buf[i];
@@ -155,10 +160,15 @@ od(FILE *fp, char *fname, int last)
}
}
}
+ if (n < 0) {
+ weprintf("read %s:", fname);
+ return n;
+ }
if (lineoff && last)
printline(line, lineoff, addr - lineoff);
if (last)
printline((unsigned char *)"", 0, addr);
+ return 0;
}

static int
@@ -196,7 +206,7 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE *fp;
+ int fd;
struct type *t;
int ret = 0, len;
char *s;
@@ -293,25 +303,26 @@ main(int argc, char *argv[])
linelen *= 2;

if (!argc) {
- od(stdin, "<stdin>", 1);
+ if (od(0, "<stdin>", 1) < 0)
+ ret = 1;
} else {
for (; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- od(fp, *argv, (!*(argv + 1)));
- if (fp != stdin && fshut(fp, *argv))
+ if (od(fd, *argv, (!*(argv + 1))) < 0)
ret = 1;
+ if (fd != 0)
+ close(fd);
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>") |
- fshut(stderr, "<stderr>");
+ ret |= fshut(stdout, "<stdout>") | fshut(stderr, "<stderr>");

return ret;
}
diff --git a/tee.c b/tee.c
index 35e3db5..eac106c 100644
--- a/tee.c
+++ b/tee.c
@@ -1,6 +1,7 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <signal.h>
-#include <stdio.h>
+#include <unistd.h>

#include "util.h"

@@ -13,14 +14,15 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE **fps = NULL;
- size_t i, n, nfps;
- int ret = 0, aflag = 0, iflag = 0;
+ int *fds = NULL;
+ size_t i, nfds;
+ ssize_t n;
+ int ret = 0, aflag = O_TRUNC, iflag = 0;
char buf[BUFSIZ];

ARGBEGIN {
case 'a':
- aflag = 1;
+ aflag = O_APPEND;
break;
case 'i':
iflag = 1;
@@ -31,31 +33,28 @@ main(int argc, char *argv[])

if (iflag && signal(SIGINT, SIG_IGN) == SIG_ERR)
eprintf("signal:");
- nfps = argc + 1;
- fps = ecalloc(nfps, sizeof(*fps));
+ nfds = argc + 1;
+ fds = ecalloc(nfds, sizeof(*fds));

for (i = 0; i < argc; i++) {
- if (!(fps[i] = fopen(argv[i], aflag ? "a" : "w"))) {
- weprintf("fopen %s:", argv[i]);
+ if ((fds[i] = open(argv[i], O_WRONLY|O_CREAT|aflag, 0666)) < 0) {
+ weprintf("open %s:", argv[i]);
ret = 1;
}
}
- fps[i] = stdout;
+ fds[i] = 1;

- while ((n = fread(buf, 1, sizeof(buf), stdin))) {
- for (i = 0; i < nfps; i++) {
- if (fps[i] && fwrite(buf, 1, n, fps[i]) != n) {
- fshut(fps[i], (i != argc) ? argv[i] : "<stdout>");
- fps[i] = NULL;
+ while ((n = read(0, buf, sizeof(buf))) > 0) {
+ for (i = 0; i < nfds; i++) {
+ if (fds[i] >= 0 && writeall(fds[i], buf, n) < 0) {
+ weprintf("write %s:", (i != argc) ? argv[i] : "<stdout>");
+ fds[i] = -1;
ret = 1;
}
}
}
-
- ret |= fshut(stdin, "<stdin>");
- for (i = 0; i < nfps; i++)
- if (fps[i])
- ret |= fshut(fps[i], (i != argc) ? argv[i] : "<stdout>");
+ if (n < 0)
+ eprintf("read <stdin>:");

return ret;
}
--
2.10.2
Laslo Hunhold
2016-12-27 13:36:42 UTC
Permalink
On Tue, 6 Dec 2016 02:16:56 -0800
Michael Forney <***@mforney.org> wrote:

Hey Michael,
Post by Michael Forney
fread reads the entire requested size (BUFSIZ), which causes tools to
block if only small amounts of data are available at a time. At best,
this causes unnecessary copies and inefficiency, at worst, tools like
tee and cat are almost unusable in some cases since they only display
large chunks of data at a time.
sbase uses FILE-pointers exclusively and mixing both "logics" is not
very helpful for the clarity of the expressed code.
The writeall() function looks like an util function that could be
introduced to completely convert sbase from fp to fd stream handling.

I think this is not the place here to discuss this, but I would be glad
if you could create a new thread here on hackers@ where you give some
examples where buffered IO is problematic.
The necessity to add functions like fshut() really made me think twice
about the advantages of buffered IO.

Cheers

Laslo
--
Laslo Hunhold <***@frign.de>
Michael Forney
2016-12-06 10:16:57 UTC
Permalink
---
xinstall.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xinstall.c b/xinstall.c
index bf921fb..5a0e390 100644
--- a/xinstall.c
+++ b/xinstall.c
@@ -119,7 +119,8 @@ install(const char *s1, const char *s2, int depth)
}
concat(f1, s1, f2, s2);

- fchmod(fileno(f2), mode);
+ if (fchmod(fileno(f2), mode) < 0)
+ eprintf("fchmod %s:", s2);

if (fclose(f2) == EOF)
eprintf("fclose %s:", s2);
--
2.10.2
Laslo Hunhold
2016-12-27 13:37:42 UTC
Permalink
On Tue, 6 Dec 2016 02:16:57 -0800
Michael Forney <***@mforney.org> wrote:

Hey Michael,
good catch! Applied.

Cheers

Laslo
--
Laslo Hunhold <***@frign.de>
Michael Forney
2016-12-06 10:16:58 UTC
Permalink
If we are just copying data from one file to another, we don't need to
fill a complete buffer, just read a chunk at a time, and write it to the
output.
---
cat.c | 39 +++++------
libutil/concat.c | 24 ++++---
libutil/cp.c | 42 +++++-------
sponge.c | 31 +++++----
tail.c | 199 +++++++++++++++++++++++++++++++++----------------------
text.h | 1 -
util.h | 1 +
xinstall.c | 25 ++++---
8 files changed, 196 insertions(+), 166 deletions(-)

diff --git a/cat.c b/cat.c
index e3741aa..211e8d1 100644
--- a/cat.c
+++ b/cat.c
@@ -1,22 +1,11 @@
/* See LICENSE file for copyright and license details. */
-#include <stdio.h>
+#include <fcntl.h>
#include <string.h>
#include <unistd.h>

-#include "text.h"
#include "util.h"

static void
-uconcat(FILE *fp1, const char *s1, FILE *fp2, const char *s2)
-{
- int c;
-
- setbuf(fp2, NULL);
- while ((c = getc(fp1)) != EOF)
- putc(c, fp2);
-}
-
-static void
usage(void)
{
eprintf("usage: %s [-u] [file ...]\n", argv0);
@@ -25,37 +14,39 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE *fp;
- int ret = 0;
- void (*cat)(FILE *, const char *, FILE *, const char *) = &concat;
+ int fd, ret = 0;

ARGBEGIN {
case 'u':
- cat = &uconcat;
break;
default:
usage();
} ARGEND

if (!argc) {
- cat(stdin, "<stdin>", stdout, "<stdout>");
+ if (concat(0, "<stdin>", 1, "<stdout>") < 0)
+ ret = 1;
} else {
for (; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- cat(fp, *argv, stdout, "<stdout>");
- if (fp != stdin && fshut(fp, *argv))
+ switch (concat(fd, *argv, 1, "<stdout>")) {
+ case -1:
ret = 1;
+ break;
+ case -2:
+ return 1; /* exit on write error */
+ }
+ if (fd != 0)
+ close(fd);
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>");
-
return ret;
}
diff --git a/libutil/concat.c b/libutil/concat.c
index fad9471..2e9aa52 100644
--- a/libutil/concat.c
+++ b/libutil/concat.c
@@ -1,19 +1,23 @@
/* See LICENSE file for copyright and license details. */
-#include <stdio.h>
+#include <unistd.h>

-#include "../text.h"
#include "../util.h"

-void
-concat(FILE *fp1, const char *s1, FILE *fp2, const char *s2)
+int
+concat(int f1, const char *s1, int f2, const char *s2)
{
char buf[BUFSIZ];
- size_t n;
+ ssize_t n;

- while ((n = fread(buf, 1, sizeof(buf), fp1))) {
- fwrite(buf, 1, n, fp2);
-
- if (feof(fp1) || ferror(fp1) || ferror(fp2))
- break;
+ while ((n = read(f1, buf, sizeof(buf))) > 0) {
+ if (writeall(f2, buf, n) < 0) {
+ weprintf("write %s:", s2);
+ return -2;
+ }
+ }
+ if (n < 0) {
+ weprintf("read %s:", s1);
+ return -1;
}
+ return 0;
}
diff --git a/libutil/cp.c b/libutil/cp.c
index c398962..8cd0a7d 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -12,7 +12,6 @@
#include <utime.h>

#include "../fs.h"
-#include "../text.h"
#include "../util.h"

int cp_aflag = 0;
@@ -27,7 +26,7 @@ int
cp(const char *s1, const char *s2, int depth)
{
DIR *dp;
- FILE *f1, *f2;
+ int f1, f2;
struct dirent *d;
struct stat st;
struct timespec times[2];
@@ -112,43 +111,34 @@ cp(const char *s1, const char *s2, int depth)
return 0;
}
} else {
- if (!(f1 = fopen(s1, "r"))) {
- weprintf("fopen %s:", s1);
+ if ((f1 = open(s1, O_RDONLY)) < 0) {
+ weprintf("open %s:", s1);
cp_status = 1;
return 0;
}
- if (!(f2 = fopen(s2, "w"))) {
- if (cp_fflag) {
- if (unlink(s2) < 0 && errno != ENOENT) {
- weprintf("unlink %s:", s2);
- cp_status = 1;
- return 0;
- } else if (!(f2 = fopen(s2, "w"))) {
- weprintf("fopen %s:", s2);
- cp_status = 1;
- return 0;
- }
- } else {
- weprintf("fopen %s:", s2);
+ if ((f2 = creat(s2, st.st_mode)) < 0 && cp_fflag) {
+ if (unlink(s2) < 0 && errno != ENOENT) {
+ weprintf("unlink %s:", s2);
cp_status = 1;
return 0;
}
+ f2 = creat(s2, st.st_mode);
}
- concat(f1, s1, f2, s2);
-
- /* preserve permissions by default */
- fchmod(fileno(f2), st.st_mode);
-
- if (fclose(f2) == EOF) {
- weprintf("fclose %s:", s2);
+ if (f2 < 0) {
+ weprintf("creat %s:", s2);
cp_status = 1;
return 0;
}
- if (fclose(f1) == EOF) {
- weprintf("fclose %s:", s1);
+ if (concat(f1, s1, f2, s2) < 0) {
cp_status = 1;
return 0;
}
+
+ /* preserve permissions by default */
+ fchmod(f2, st.st_mode);
+
+ close(f1);
+ close(f2);
}

if (cp_aflag || cp_pflag) {
diff --git a/sponge.c b/sponge.c
index baeac7f..da8b28c 100644
--- a/sponge.c
+++ b/sponge.c
@@ -1,7 +1,8 @@
/* See LICENSE file for copyright and license details. */
-#include <stdio.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>

-#include "text.h"
#include "util.h"

static void
@@ -13,24 +14,26 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE *fp, *tmpfp;
- int ret = 0;
+ char tmp[] = "/tmp/sponge-XXXXXX";
+ int fd, tmpfd;

argv0 = argv[0], argc--, argv++;

if (argc != 1)
usage();

- if (!(tmpfp = tmpfile()))
- eprintf("tmpfile:");
- concat(stdin, "<stdin>", tmpfp, "<tmpfile>");
- rewind(tmpfp);
+ if ((tmpfd = mkstemp(tmp)) < 0)
+ eprintf("mkstemp:");
+ unlink(tmp);
+ if (concat(0, "<stdin>", tmpfd, "<tmpfile>") < 0)
+ return 1;
+ if (lseek(tmpfd, 0, SEEK_SET) < 0)
+ eprintf("lseek:");

- if (!(fp = fopen(argv[0], "w")))
- eprintf("fopen %s:", argv[0]);
- concat(tmpfp, "<tmpfile>", fp, argv[0]);
+ if ((fd = creat(argv[0], 0666)) < 0)
+ eprintf("creat %s:", argv[0]);
+ if (concat(tmpfd, "<tmpfile>", fd, argv[0]) < 0)
+ return 1;

- ret |= fshut(fp, argv[0]) | fshut(tmpfp, "<tmpfile>");
-
- return ret;
+ return 0;
}
diff --git a/tail.c b/tail.c
index 711707f..1ab9d18 100644
--- a/tail.c
+++ b/tail.c
@@ -1,80 +1,125 @@
/* See LICENSE file for copyright and license details. */
#include <sys/stat.h>

+#include <fcntl.h>
+#include <unistd.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

-#include "text.h"
#include "utf.h"
#include "util.h"

static char mode = 'n';

-static void
-dropinit(FILE *fp, const char *str, size_t n)
+static int
+dropinit(int fd, const char *fname, size_t count)
{
Rune r;
- char *buf = NULL;
- size_t size = 0, i = 1;
- ssize_t len;
+ char buf[BUFSIZ], *p;
+ ssize_t n;
+ int nr;
+
+ if (count < 2)
+ goto copy;
+ count--; /* numbering starts at 1 */
+ while (count && (n = read(fd, buf, sizeof(buf))) > 0) {
+ if (mode == 'n') {
+ for (p = buf; count && n > 0; p++, n--) {
+ if (*p == '\n')
+ count--;
+ }
+ } else {
+ for (p = buf; count && n > 0; p += nr, n -= nr, count--) {
+ nr = charntorune(&r, p, n);
+ if (!nr) {
+ /* we don't have a full rune, move
+ * remaining data to beginning and read
+ * again */
+ memmove(buf, p, n);
+ break;
+ }
+ }
+ }
+ }
+ if (count) {
+ if (n < 0)
+ weprintf("read %s:", fname);
+ if (n <= 0)
+ return n;
+ }

- if (mode == 'n') {
- while (i < n && (len = getline(&buf, &size, fp)) > 0)
- if (len > 0 && buf[len - 1] == '\n')
- i++;
- } else {
- while (i < n && efgetrune(&r, fp, str))
- i++;
+ /* write the rest of the buffer */
+ if (writeall(1, p, n) < 0)
+ eprintf("write:");
+copy:
+ switch (concat(fd, fname, 1, "<stdout>")) {
+ case -1: /* read error */
+ return -1;
+ case -2: /* write error */
+ exit(1);
+ default:
+ return 0;
}
- free(buf);
- concat(fp, str, stdout, "<stdout>");
}

-static void
-taketail(FILE *fp, const char *str, size_t n)
+static int
+taketail(int fd, const char *fname, size_t count)
{
- Rune *r = NULL;
- struct line *ring = NULL;
- size_t i, j, *size = NULL;
- ssize_t len;
- int seenln = 0;
-
- if (!n)
- return;
-
- if (mode == 'n') {
- ring = ecalloc(n, sizeof(*ring));
- size = ecalloc(n, sizeof(*size));
-
- for (i = j = 0; (len = getline(&ring[i].data,
- &size[i], fp)) > 0; seenln = 1) {
- ring[i].len = len;
- i = j = (i + 1) % n;
+ static char *buf = NULL;
+ static size_t size = 0;
+ char *p;
+ size_t len = 0, left;
+ ssize_t n;
+
+ if (!count)
+ return 0;
+ for (;;) {
+ if (len + BUFSIZ > size) {
+ /* make sure we have at least BUFSIZ to read */
+ size += 2 * BUFSIZ;
+ buf = erealloc(buf, size);
}
- } else {
- r = ecalloc(n, sizeof(*r));
-
- for (i = j = 0; efgetrune(&r[i], fp, str); )
- i = j = (i + 1) % n;
- }
- if (ferror(fp))
- eprintf("%s: read error:", str);
-
- do {
- if (seenln && ring && ring[j].data) {
- fwrite(ring[j].data, 1, ring[j].len, stdout);
- free(ring[j].data);
- } else if (r) {
- efputrune(&r[j], stdout, "<stdout>");
+ n = read(fd, buf + len, size - len);
+ if (n < 0) {
+ weprintf("read %s:", fname);
+ return -1;
}
- } while ((j = (j + 1) % n) != i);
-
- free(ring);
- free(size);
- free(r);
+ if (n == 0)
+ break;
+ len += n;
+ if (mode == 'n') {
+ /* ignore the last character; if it is a newline, it
+ * ends the last line */
+ for (p = buf + len - 2, left = count; p >= buf; p--) {
+ if (*p != '\n')
+ continue;
+ left--;
+ if (!left) {
+ p++;
+ break;
+ }
+ }
+ } else {
+ for (p = buf + len - 1, left = count; p >= buf; p--) {
+ /* skip utf-8 continuation bytes */
+ if ((*p & 0xc0) == 0x80)
+ continue;
+ left--;
+ if (!left)
+ break;
+ }
+ }
+ if (p > buf) {
+ len -= p - buf;
+ memmove(buf, p, len);
+ }
+ }
+ if (writeall(1, buf, len) < 0)
+ eprintf("write:");
+ return 0;
}

static void
@@ -87,11 +132,11 @@ int
main(int argc, char *argv[])
{
struct stat st1, st2;
- FILE *fp;
- size_t tmpsize, n = 10;
+ int fd;
+ size_t n = 10;
int fflag = 0, ret = 0, newline = 0, many = 0;
- char *numstr, *tmp;
- void (*tail)(FILE *, const char *, size_t) = taketail;
+ char *numstr;
+ int (*tail)(int, const char *, size_t) = taketail;

ARGBEGIN {
case 'f':
@@ -113,17 +158,18 @@ main(int argc, char *argv[])
usage();
} ARGEND

- if (!argc)
- tail(stdin, "<stdin>", n);
- else {
+ if (!argc) {
+ if (tail(0, "<stdin>", n) < 0)
+ ret = 1;
+ } else {
if ((many = argc > 1) && fflag)
usage();
for (newline = 0; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
@@ -134,27 +180,26 @@ main(int argc, char *argv[])
if (!(S_ISFIFO(st1.st_mode) || S_ISREG(st1.st_mode)))
fflag = 0;
newline = 1;
- tail(fp, *argv, n);
+ if (tail(fd, *argv, n) < 0) {
+ ret = 1;
+ fflag = 0;
+ }

if (!fflag) {
- if (fp != stdin && fshut(fp, *argv))
- ret = 1;
+ if (fd != 0)
+ close(fd);
continue;
}
- for (tmp = NULL, tmpsize = 0;;) {
- while (getline(&tmp, &tmpsize, fp) > 0) {
- fputs(tmp, stdout);
- fflush(stdout);
- }
- if (ferror(fp))
- eprintf("readline %s:", *argv);
- clearerr(fp);
+ for (;;) {
+ if (concat(fd, *argv, 1, "<stdout>") < 0)
+ exit(1);
/* ignore error in case file was removed, we continue
* tracking the existing open file descriptor */
if (!stat(*argv, &st2)) {
if (st2.st_size < st1.st_size) {
fprintf(stderr, "%s: file truncated\n", *argv);
- rewind(fp);
+ if (lseek(fd, SEEK_SET, 0) < 0)
+ eprintf("lseek:");
}
st1 = st2;
}
@@ -163,7 +208,5 @@ main(int argc, char *argv[])
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>");
-
return ret;
}
diff --git a/text.h b/text.h
index bceda52..9858592 100644
--- a/text.h
+++ b/text.h
@@ -13,5 +13,4 @@ struct linebuf {
#define EMPTY_LINEBUF {NULL, 0, 0,}
void getlines(FILE *, struct linebuf *);

-void concat(FILE *, const char *, FILE *, const char *);
int linecmp(struct line *, struct line *);
diff --git a/util.h b/util.h
index eaad3ce..968a3af 100644
--- a/util.h
+++ b/util.h
@@ -64,6 +64,7 @@ int eregcomp(regex_t *, const char *, int);

/* io */
ssize_t writeall(int, const void *, size_t);
+int concat(int, const char *, int, const char *);

/* misc */
void enmasse(int, char **, int (*)(const char *, const char *, int));
diff --git a/xinstall.c b/xinstall.c
index 5a0e390..3d24a1a 100644
--- a/xinstall.c
+++ b/xinstall.c
@@ -2,6 +2,7 @@
#include <grp.h>
#include <pwd.h>
#include <errno.h>
+#include <fcntl.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
@@ -10,7 +11,6 @@
#include <sys/wait.h>

#include "util.h"
-#include "text.h"

static int Dflag = 0;
static int sflag = 0;
@@ -61,7 +61,7 @@ static int
install(const char *s1, const char *s2, int depth)
{
DIR *dp;
- FILE *f1, *f2;
+ int f1, f2;
struct dirent *d;
struct stat st;
ssize_t r;
@@ -109,23 +109,22 @@ install(const char *s1, const char *s2, int depth)
else if (mknod(s2, (st.st_mode & ~07777) | mode, st.st_rdev) < 0)
eprintf("mknod %s:", s2);
} else {
- if (!(f1 = fopen(s1, "r")))
- eprintf("fopen %s:", s1);
- if (!(f2 = fopen(s2, "w"))) {
+ if ((f1 = open(s1, O_RDONLY)) < 0)
+ eprintf("open %s:", s1);
+ if ((f2 = creat(s2, 0600)) < 0) {
if (unlink(s2) < 0 && errno != ENOENT)
eprintf("unlink %s:", s2);
- else if (!(f2 = fopen(s2, "w")))
- eprintf("fopen %s:", s2);
+ if ((f2 = creat(s2, 0600)) < 0)
+ eprintf("creat %s:", s2);
}
- concat(f1, s1, f2, s2);
+ if (concat(f1, s1, f2, s2) < 0)
+ exit(1);

- if (fchmod(fileno(f2), mode) < 0)
+ if (fchmod(f2, mode) < 0)
eprintf("fchmod %s:", s2);

- if (fclose(f2) == EOF)
- eprintf("fclose %s:", s2);
- if (fclose(f1) == EOF)
- eprintf("fclose %s:", s1);
+ close(f1);
+ close(f2);

if (sflag)
strip(s2);
--
2.10.2
Michael Forney
2016-12-06 10:16:59 UTC
Permalink
Previously, when the destination file was created with fopen, we needed
to use fchmod to set its permissions.

Now that we pass in the mode to creat, we already get the desired
behavior of creating the file with the same mode as the source file
modified by the user's file creation mask.

This fixes the issue where a directory or special file created with
mkdir/mknod does not end up with the appropriate mode with -p or -a
(since it may have been narrowed by the umask).

This also allows us to clear the SUID and SGID bits from the mode if the
chown fails, as specified by POSIX.
---
libutil/cp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libutil/cp.c b/libutil/cp.c
index 8cd0a7d..339c892 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -134,15 +134,11 @@ cp(const char *s1, const char *s2, int depth)
return 0;
}

- /* preserve permissions by default */
- fchmod(f2, st.st_mode);
-
close(f1);
close(f2);
}

if (cp_aflag || cp_pflag) {
- /* timestamp and owner */
if (!S_ISLNK(st.st_mode)) {
times[0] = st.st_atim;
times[1] = st.st_mtim;
@@ -151,7 +147,11 @@ cp(const char *s1, const char *s2, int depth)
if (chown(s2, st.st_uid, st.st_gid) < 0) {
weprintf("chown %s:", s2);
cp_status = 1;
- return 0;
+ st.st_mode &= ~(S_ISUID | S_ISGID);
+ }
+ if (chmod(s2, st.st_mode) < 0) {
+ weprintf("chmod %s:", s2);
+ cp_status = 1;
}
} else {
if (lchown(s2, st.st_uid, st.st_gid) < 0) {
--
2.10.2
Michael Forney
2016-12-06 10:17:00 UTC
Permalink
---
tail.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/tail.c b/tail.c
index 1ab9d18..ce65a01 100644
--- a/tail.c
+++ b/tail.c
@@ -175,8 +175,8 @@ main(int argc, char *argv[])
}
if (many)
printf("%s==> %s <==\n", newline ? "\n" : "", *argv);
- if (stat(*argv, &st1) < 0)
- eprintf("stat %s:", *argv);
+ if (fstat(fd, &st1) < 0)
+ eprintf("fstat %s:", *argv);
if (!(S_ISFIFO(st1.st_mode) || S_ISREG(st1.st_mode)))
fflag = 0;
newline = 1;
@@ -193,16 +193,14 @@ main(int argc, char *argv[])
for (;;) {
if (concat(fd, *argv, 1, "<stdout>") < 0)
exit(1);
- /* ignore error in case file was removed, we continue
- * tracking the existing open file descriptor */
- if (!stat(*argv, &st2)) {
- if (st2.st_size < st1.st_size) {
- fprintf(stderr, "%s: file truncated\n", *argv);
- if (lseek(fd, SEEK_SET, 0) < 0)
- eprintf("lseek:");
- }
- st1 = st2;
+ if (fstat(fd, &st2) < 0)
+ eprintf("fstat %s:", *argv);
+ if (st2.st_size < st1.st_size) {
+ fprintf(stderr, "%s: file truncated\n", *argv);
+ if (lseek(fd, SEEK_SET, 0) < 0)
+ eprintf("lseek:");
}
+ st1 = st2;
sleep(1);
}
}
--
2.10.2
Michael Forney
2016-12-06 10:17:01 UTC
Permalink
POSIX says that if duplicating the modification/access times fails, then
an error should be written to stderr.
---
libutil/cp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libutil/cp.c b/libutil/cp.c
index 339c892..15e4ce5 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -142,8 +142,10 @@ cp(const char *s1, const char *s2, int depth)
if (!S_ISLNK(st.st_mode)) {
times[0] = st.st_atim;
times[1] = st.st_mtim;
- utimensat(AT_FDCWD, s2, times, 0);
-
+ if (utimensat(AT_FDCWD, s2, times, 0) < 0) {
+ weprintf("utimensat %s:", s2);
+ cp_status = 1;
+ }
if (chown(s2, st.st_uid, st.st_gid) < 0) {
weprintf("chown %s:", s2);
cp_status = 1;
--
2.10.2
Laslo Hunhold
2016-12-27 13:46:33 UTC
Permalink
On Tue, 6 Dec 2016 02:17:01 -0800
Michael Forney <***@mforney.org> wrote:

Hey Michael,
Post by Michael Forney
POSIX says that if duplicating the modification/access times fails,
then an error should be written to stderr.
thanks, applied!

Cheers

Laslo
--
Laslo Hunhold <***@frign.de>
Michael Forney
2016-12-06 10:17:02 UTC
Permalink
---
libutil/cp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libutil/cp.c b/libutil/cp.c
index 15e4ce5..f89319d 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -139,13 +139,13 @@ cp(const char *s1, const char *s2, int depth)
}

if (cp_aflag || cp_pflag) {
+ times[0] = st.st_atim;
+ times[1] = st.st_mtim;
+ if (utimensat(AT_FDCWD, s2, times, AT_SYMLINK_NOFOLLOW) < 0) {
+ weprintf("utimensat %s:", s2);
+ cp_status = 1;
+ }
if (!S_ISLNK(st.st_mode)) {
- times[0] = st.st_atim;
- times[1] = st.st_mtim;
- if (utimensat(AT_FDCWD, s2, times, 0) < 0) {
- weprintf("utimensat %s:", s2);
- cp_status = 1;
- }
if (chown(s2, st.st_uid, st.st_gid) < 0) {
weprintf("chown %s:", s2);
cp_status = 1;
--
2.10.2
Laslo Hunhold
2016-12-27 13:51:24 UTC
Permalink
On Tue, 6 Dec 2016 02:17:02 -0800
Michael Forney <***@mforney.org> wrote:

Hey Michael,
applied with a small modification. Thanks!

Cheers

Laslo
--
Laslo Hunhold <***@frign.de>
Michael Forney
2016-12-06 10:17:03 UTC
Permalink
POSIX says that -c specifies a number of bytes, not characters. This
flag is commonly used by scripts that operate on binary files to do
things like extract a header. Treating the offsets as character offsets
will break things in mysterious ways.

Instead, add a -m option (chosen to match `wc -m`, which also operates
on characters) to handle character offsets.
---
I'm tempted to just delete the character functionality instead of
introducing a new non-standard option. I can see the use of tail with
codepoints, but we definitely need to make -c work on bytes so that we
don't break scripts.

I'm also open to changing the option flag to something else. I just
chose -m because that's what wc uses for characters.

tail.1 | 6 +++---
tail.c | 29 ++++++++++++++++++++++++-----
2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/tail.1 b/tail.1
index 433404d..281560d 100644
--- a/tail.1
+++ b/tail.1
@@ -7,7 +7,7 @@
.Sh SYNOPSIS
.Nm
.Op Fl f
-.Op Fl c Ar num | Fl n Ar num | Fl Ns Ar num
+.Op Fl c Ar num | Fl m Ar num | Fl n Ar num | Fl Ns Ar num
.Op Ar file ...
.Sh DESCRIPTION
.Nm
@@ -20,10 +20,10 @@ is given,
reads from stdin.
.Sh OPTIONS
.Bl -tag -width Ds
-.It Fl c Ar num | Fl n Ar num | Fl Ns Ar num
+.It Fl c Ar num | Fl m Ar num | Fl n Ar num | Fl Ns Ar num
Display final
.Ar num
-characters | lines |
+bytes | characters | lines |
lines. If
.Ar num
begins with '+'
diff --git a/tail.c b/tail.c
index ce65a01..ce3be9d 100644
--- a/tail.c
+++ b/tail.c
@@ -26,12 +26,23 @@ dropinit(int fd, const char *fname, size_t count)
goto copy;
count--; /* numbering starts at 1 */
while (count && (n = read(fd, buf, sizeof(buf))) > 0) {
- if (mode == 'n') {
+ switch (mode) {
+ case 'n': /* lines */
for (p = buf; count && n > 0; p++, n--) {
if (*p == '\n')
count--;
}
- } else {
+ break;
+ case 'c': /* bytes */
+ if (count > n) {
+ count -= n;
+ } else {
+ p = buf + count;
+ n -= count;
+ count = 0;
+ }
+ break;
+ case 'm': /* runes */
for (p = buf; count && n > 0; p += nr, n -= nr, count--) {
nr = charntorune(&r, p, n);
if (!nr) {
@@ -42,6 +53,7 @@ dropinit(int fd, const char *fname, size_t count)
break;
}
}
+ break;
}
}
if (count) {
@@ -90,7 +102,8 @@ taketail(int fd, const char *fname, size_t count)
if (n == 0)
break;
len += n;
- if (mode == 'n') {
+ switch (mode) {
+ case 'n': /* lines */
/* ignore the last character; if it is a newline, it
* ends the last line */
for (p = buf + len - 2, left = count; p >= buf; p--) {
@@ -102,7 +115,11 @@ taketail(int fd, const char *fname, size_t count)
break;
}
}
- } else {
+ break;
+ case 'c': /* bytes */
+ p = count < len ? buf + len - count : buf;
+ break;
+ case 'm': /* runes */
for (p = buf + len - 1, left = count; p >= buf; p--) {
/* skip utf-8 continuation bytes */
if ((*p & 0xc0) == 0x80)
@@ -111,6 +128,7 @@ taketail(int fd, const char *fname, size_t count)
if (!left)
break;
}
+ break;
}
if (p > buf) {
len -= p - buf;
@@ -125,7 +143,7 @@ taketail(int fd, const char *fname, size_t count)
static void
usage(void)
{
- eprintf("usage: %s [-f] [-c num | -n num | -num] [file ...]\n", argv0);
+ eprintf("usage: %s [-f] [-c num | -m num | -n num | -num] [file ...]\n", argv0);
}

int
@@ -143,6 +161,7 @@ main(int argc, char *argv[])
fflag = 1;
break;
case 'c':
+ case 'm':
case 'n':
mode = ARGC();
numstr = EARGF(usage());
--
2.10.2
Laslo Hunhold
2016-12-27 13:55:42 UTC
Permalink
On Tue, 6 Dec 2016 02:17:03 -0800
Michael Forney <***@mforney.org> wrote:

Hey Michael,
Post by Michael Forney
POSIX says that -c specifies a number of bytes, not characters. This
flag is commonly used by scripts that operate on binary files to do
things like extract a header. Treating the offsets as character
offsets will break things in mysterious ways.
Instead, add a -m option (chosen to match `wc -m`, which also operates
on characters) to handle character offsets.
---
I'm tempted to just delete the character functionality instead of
introducing a new non-standard option. I can see the use of tail with
codepoints, but we definitely need to make -c work on bytes so that we
don't break scripts.
I'm also open to changing the option flag to something else. I just
chose -m because that's what wc uses for characters.
well-spotted! Still, it's _very_ counterintuitive to call the flag
"-c". Instead of adding a non-portable m-flag, it would even sound
better to me to add a b-flag for byte-offsets.

It all depends on how many scripts rely on this behaviour. Can you give
an example? I thought cut(1) was the tool of choice for extracting
headers and such things.

Cheers

Laslo
--
Laslo Hunhold <***@frign.de>
Evan Gates
2016-12-27 17:41:09 UTC
Permalink
Post by Laslo Hunhold
well-spotted! Still, it's _very_ counterintuitive to call the flag
"-c". Instead of adding a non-portable m-flag, it would even sound
better to me to add a b-flag for byte-offsets.
It all depends on how many scripts rely on this behaviour. Can you give
an example? I thought cut(1) was the tool of choice for extracting
headers and such things.
I think deviating from POSIX here is a bad idea. Every deviation from
POSIX means that our tools cannot be used in another situation and
pushes prospective users away. If the user wants characters instead of
bytes we have tools to do that, don't surprise the user by doing
something different than every other implementation.

P.S. I too found -c confusing the first time I expected utf8
characters, but remembering these tools were created with ascii in
mind, I think of -c as char and it all works out...
Michael Forney
2016-12-28 02:03:24 UTC
Permalink
Post by Evan Gates
Post by Laslo Hunhold
well-spotted! Still, it's _very_ counterintuitive to call the flag
"-c". Instead of adding a non-portable m-flag, it would even sound
better to me to add a b-flag for byte-offsets.
Yes, it's a bit counter-intuitive, but conflicting with POSIX for this
alone seems like a really bad idea. I always consult POSIX when
writing shell scripts to ensure that they will run on any conforming
system. If sbase decided that the option character name was not the
best choice, then reasonable, valid, and portable scripts may start
operating unexpectedly with no indication as to why.

Also, wc(1) (even sbase's implementation) uses -c to refer to bytes,
and -m to refer to characters. It wouldn't be self-consistent to make
tail use -b for bytes and -c for characters. (Just to clarify, I also
think it would be a really bad idea to make wc use -b for bytes and -c
for characters).
Post by Evan Gates
Post by Laslo Hunhold
It all depends on how many scripts rely on this behaviour. Can you give
an example?
Sure. gcc's build system uses tail to skip the first 16 bytes of the
binaries to check that stage2 and stage3 are the same. Granted, it
does use non-standard syntax tail +16c, and I don't know that there
are any bytes in there with the high bit set, but still, tail *does*
get invoked on binary files, and treating the byte offsets as
characters will break things in strange ways that are difficult to
debug.
Post by Evan Gates
Post by Laslo Hunhold
I thought cut(1) was the tool of choice for extracting
headers and such things.
How do you use cut(1) to strip the first 512 bytes of a binary file?
It operates on lines.
Post by Evan Gates
I think deviating from POSIX here is a bad idea. Every deviation from
POSIX means that our tools cannot be used in another situation and
pushes prospective users away. If the user wants characters instead of
bytes we have tools to do that, don't surprise the user by doing
something different than every other implementation.
P.S. I too found -c confusing the first time I expected utf8
characters, but remembering these tools were created with ascii in
mind, I think of -c as char and it all works out...
Agreed.
Laslo Hunhold
2017-01-02 20:13:19 UTC
Permalink
On Tue, 27 Dec 2016 18:03:24 -0800
Michael Forney <***@mforney.org> wrote:

Hey Michael,
yes, the reasoning makes sense. Could you please update your patch so it
applies on git HEAD?

Cheers

Laslo
--
Laslo Hunhold <***@frign.de>
Michael Forney
2017-01-02 01:00:31 UTC
Permalink
Post by Laslo Hunhold
On Tue, 6 Dec 2016 02:16:56 -0800
Hey Michael,
Post by Michael Forney
fread reads the entire requested size (BUFSIZ), which causes tools to
block if only small amounts of data are available at a time. At best,
this causes unnecessary copies and inefficiency, at worst, tools like
tee and cat are almost unusable in some cases since they only display
large chunks of data at a time.
sbase uses FILE-pointers exclusively and mixing both "logics" is not
very helpful for the clarity of the expressed code.
fread and read behave differently and have different use cases. I don't
see a problem with using whichever API is best suited for the task.
Post by Laslo Hunhold
The writeall() function looks like an util function that could be
introduced to completely convert sbase from fp to fd stream handling.
I don't think writeall will help at all with reading files line-by-line
as many of the tools require.
Post by Laslo Hunhold
I think this is not the place here to discuss this, but I would be glad
examples where buffered IO is problematic.
I feel like I have already justified this change enough in previous
comments.

However, I will take this rebase patch set as an opportunity to sneak in
one final response.

It is impossible to implement tee or cat -u correctly with fread unless
you read one byte at a time.

cat -u should "Write bytes from the input file to the standard output
without delay as each is read". If you fread into a BUFSIZ byte buffer,
it will call read successively until the buffer is filled or EOF is hit,
delaying the write.

tee "shall not buffer output", but by reading into intermediate buffer
with fread before the fwrite, we are buffering the output.

setbuf and setvbuf are not relevant because the problem is with the
fread behavior of returning "the number of elements successfully read
which is less than nitems only if a read error or end-of-file is
encountered".

The current implementation makes cat and tee not very useful when used
in a pipeline connected to a tty.

This is why nobody implements these utilities with fread:
- toybox (https://github.com/landley/toybox/blob/master/toys/posix/cat.c#L59)
- busybox (https://git.busybox.net/busybox/tree/libbb/copyfd.c#n88)
- openbsd (http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/cat/cat.c?annotate=1.26)
- coreutils (http://git.savannah.gnu.org/cgit/coreutils.git/tree/src/cat.c#n342)

You could argue that we should just use fwrite instead of writeall, but
I think writeall is simple enough, and avoids mixing file descriptors
and FILE * in concat.
Post by Laslo Hunhold
The necessity to add functions like fshut() really made me think twice
about the advantages of buffered IO.
Yes, I think stdio is a pretty unfortunate API, but replacing it
completely in sbase is a much bigger change with less benefit.

Changes since v2:
- Rebase on latest git.

Changes since v1:
- Changed concat to return -2 on write error so that callers have the option to
handle it differently.
- Added a patch to libutil/cp.c to preserve atime/mtime for symlinks.
- Changed the od overflow fix to be a bit clearer and avoid casting off_t to
size_t.
- Switched tail to use raw IO and concat as well.
- Added a patch to tail to make -c offsets refer to bytes instead of runes.

Michael Forney (6):
libutil: Add writeall utility function
Don't use buffered IO (fread) when not appropriate
concat: Use plain read/write instead of buffered stdio
cp: Only call chmod with -p or -a
tail: Use fstat in case file is removed
tail: Process bytes with -c option, and add -m option for runes

Makefile | 3 +-
cat.c | 39 ++++-----
cksum.c | 31 +++----
crypt.h | 2 +-
libutil/concat.c | 24 +++---
libutil/cp.c | 47 +++++------
libutil/crypt.c | 37 +++++----
libutil/writeall.c | 21 +++++
od.c | 43 ++++++----
sponge.c | 31 +++----
tail.1 | 6 +-
tail.c | 236 +++++++++++++++++++++++++++++++++--------------------
tee.c | 39 +++++----
text.h | 1 -
util.h | 4 +
xinstall.c | 25 +++---
16 files changed, 338 insertions(+), 251 deletions(-)
create mode 100644 libutil/writeall.c
--
2.11.0
Michael Forney
2017-01-02 01:00:32 UTC
Permalink
writeall makes successive write calls to write an entire buffer to the
output file descriptor. It returns the number of bytes written, or -1 on
the first error.
---
Makefile | 3 ++-
libutil/writeall.c | 21 +++++++++++++++++++++
util.h | 3 +++
3 files changed, 26 insertions(+), 1 deletion(-)
create mode 100644 libutil/writeall.c

diff --git a/Makefile b/Makefile
index 9ec9990..1c39fef 100644
--- a/Makefile
+++ b/Makefile
@@ -79,7 +79,8 @@ LIBUTILSRC =\
libutil/strlcpy.c\
libutil/strsep.c\
libutil/strtonum.c\
- libutil/unescape.c
+ libutil/unescape.c\
+ libutil/writeall.c

LIB = $(LIBUTF) $(LIBUTIL)

diff --git a/libutil/writeall.c b/libutil/writeall.c
new file mode 100644
index 0000000..4725ced
--- /dev/null
+++ b/libutil/writeall.c
@@ -0,0 +1,21 @@
+/* See LICENSE file for copyright and license details. */
+#include <unistd.h>
+
+#include "../util.h"
+
+ssize_t
+writeall(int fd, const void *buf, size_t len)
+{
+ const char *p = buf;
+ ssize_t n;
+
+ while (len) {
+ n = write(fd, p, len);
+ if (n <= 0)
+ return n;
+ p += n;
+ len -= n;
+ }
+
+ return p - (const char *)buf;
+}
diff --git a/util.h b/util.h
index b5860dc..eaad3ce 100644
--- a/util.h
+++ b/util.h
@@ -62,6 +62,9 @@ char *strsep(char **, const char *);
int enregcomp(int, regex_t *, const char *, int);
int eregcomp(regex_t *, const char *, int);

+/* io */
+ssize_t writeall(int, const void *, size_t);
+
/* misc */
void enmasse(int, char **, int (*)(const char *, const char *, int));
void fnck(const char *, const char *, int (*)(const char *, const char *, int), int);
--
2.11.0
Michael Forney
2017-01-02 01:00:33 UTC
Permalink
fread reads the entire requested size (BUFSIZ), which causes tools to
block if only small amounts of data are available at a time. At best,
this causes unnecessary copies and inefficiency, at worst, tools like
tee and cat are almost unusable in some cases since they only display
large chunks of data at a time.
---
cksum.c | 31 +++++++++++++++++--------------
crypt.h | 2 +-
libutil/crypt.c | 37 +++++++++++++++++++------------------
od.c | 43 +++++++++++++++++++++++++++----------------
tee.c | 39 +++++++++++++++++++--------------------
5 files changed, 83 insertions(+), 69 deletions(-)

diff --git a/cksum.c b/cksum.c
index 570ca81..b53ec17 100644
--- a/cksum.c
+++ b/cksum.c
@@ -1,7 +1,9 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <inttypes.h>
#include <stdio.h>
#include <string.h>
+#include <unistd.h>

#include "util.h"

@@ -61,19 +63,20 @@ static const unsigned long crctab[] = { 0x00000000,
};

static void
-cksum(FILE *fp, const char *s)
+cksum(int fd, const char *s)
{
- size_t len = 0, i, n;
+ ssize_t n;
+ size_t len = 0, i;
uint32_t ck = 0;
unsigned char buf[BUFSIZ];

- while ((n = fread(buf, 1, sizeof(buf), fp))) {
+ while ((n = read(fd, buf, sizeof(buf))) > 0) {
for (i = 0; i < n; i++)
ck = (ck << 8) ^ crctab[(ck >> 24) ^ buf[i]];
len += n;
}
- if (ferror(fp)) {
- weprintf("fread %s:", s ? s : "<stdin>");
+ if (n < 0) {
+ weprintf("read %s:", s ? s : "<stdin>");
ret = 1;
return;
}
@@ -92,29 +95,29 @@ cksum(FILE *fp, const char *s)
int
main(int argc, char *argv[])
{
- FILE *fp;
+ int fd;

argv0 = argv[0], argc--, argv++;

if (!argc) {
- cksum(stdin, NULL);
+ cksum(0, NULL);
} else {
for (; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- cksum(fp, *argv);
- if (fp != stdin && fshut(fp, *argv))
- ret = 1;
+ cksum(fd, *argv);
+ if (fd != 0)
+ close(fd);
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>");
+ ret |= fshut(stdout, "<stdout>");

return ret;
}
diff --git a/crypt.h b/crypt.h
index e0cc08d..2fd2932 100644
--- a/crypt.h
+++ b/crypt.h
@@ -8,5 +8,5 @@ struct crypt_ops {

int cryptcheck(int, char **, struct crypt_ops *, uint8_t *, size_t);
int cryptmain(int, char **, struct crypt_ops *, uint8_t *, size_t);
-int cryptsum(struct crypt_ops *, FILE *, const char *, uint8_t *);
+int cryptsum(struct crypt_ops *, int, const char *, uint8_t *);
void mdprint(const uint8_t *, const char *, size_t);
diff --git a/libutil/crypt.c b/libutil/crypt.c
index 6991c39..e285614 100644
--- a/libutil/crypt.c
+++ b/libutil/crypt.c
@@ -1,8 +1,10 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>

#include "../crypt.h"
#include "../text.h"
@@ -41,7 +43,7 @@ static void
mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
int *formatsucks, int *noread, int *nonmatch)
{
- FILE *fp;
+ int fd;
size_t bufsiz = 0;
int r;
char *line = NULL, *file, *p;
@@ -59,12 +61,12 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
file += 2;
for (p = file; *p && *p != '\n' && *p != '\r'; p++); /* strip newline */
*p = '\0';
- if (!(fp = fopen(file, "r"))) {
- weprintf("fopen %s:", file);
+ if ((fd = open(file, O_RDONLY)) < 0) {
+ weprintf("open %s:", file);
(*noread)++;
continue;
}
- if (cryptsum(ops, fp, file, md)) {
+ if (cryptsum(ops, fd, file, md)) {
(*noread)++;
continue;
}
@@ -77,7 +79,7 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t *md, size_t sz,
} else {
(*formatsucks)++;
}
- fclose(fp);
+ close(fd);
}
free(line);
}
@@ -124,11 +126,11 @@ cryptcheck(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz
int
cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
{
- FILE *fp;
+ int fd;
int ret = 0;

if (argc == 0) {
- if (cryptsum(ops, stdin, "<stdin>", md))
+ if (cryptsum(ops, 0, "<stdin>", md))
ret = 1;
else
mdprint(md, "<stdin>", sz);
@@ -136,18 +138,18 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
for (; *argv; argc--, argv++) {
if ((*argv)[0] == '-' && !(*argv)[1]) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- if (cryptsum(ops, fp, *argv, md))
+ if (cryptsum(ops, fd, *argv, md))
ret = 1;
else
mdprint(md, *argv, sz);
- if (fp != stdin && fshut(fp, *argv))
- ret = 1;
+ if (fd != 0)
+ close(fd);
}
}

@@ -155,16 +157,15 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, uint8_t *md, size_t sz)
}

int
-cryptsum(struct crypt_ops *ops, FILE *fp, const char *f,
- uint8_t *md)
+cryptsum(struct crypt_ops *ops, int fd, const char *f, uint8_t *md)
{
uint8_t buf[BUFSIZ];
- size_t n;
+ ssize_t n;

ops->init(ops->s);
- while ((n = fread(buf, 1, sizeof(buf), fp)) > 0)
+ while ((n = read(fd, buf, sizeof(buf))) > 0)
ops->update(ops->s, buf, n);
- if (ferror(fp)) {
+ if (n < 0) {
weprintf("%s: read error:", f);
return 1;
}
diff --git a/od.c b/od.c
index 27a7104..e5dde83 100644
--- a/od.c
+++ b/od.c
@@ -1,8 +1,10 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>

#include "queue.h"
#include "util.h"
@@ -124,20 +126,23 @@ once:
}
}

-static void
-od(FILE *fp, char *fname, int last)
+static int
+od(int fd, char *fname, int last)
{
static unsigned char *line;
static size_t lineoff;
static off_t addr;
unsigned char buf[BUFSIZ];
- size_t i, n, size = sizeof(buf);
+ size_t i, size = sizeof(buf);
+ ssize_t n;

while (skip - addr > 0) {
- n = fread(buf, 1, MIN(skip - addr, sizeof(buf)), fp);
+ n = read(fd, buf, MIN(skip - addr, sizeof(buf)));
+ if (n < 0)
+ weprintf("read %s:", fname);
+ if (n <= 0)
+ return n;
addr += n;
- if (feof(fp) || ferror(fp))
- return;
}
if (!line)
line = emalloc(linelen);
@@ -145,7 +150,7 @@ od(FILE *fp, char *fname, int last)
for (;;) {
if (max >= 0)
size = MIN(max - (addr - skip), size);
- if (!(n = fread(buf, 1, size, fp)))
+ if ((n = read(fd, buf, size)) <= 0)
break;
for (i = 0; i < n; i++, addr++) {
line[lineoff++] = buf[i];
@@ -155,10 +160,15 @@ od(FILE *fp, char *fname, int last)
}
}
}
+ if (n < 0) {
+ weprintf("read %s:", fname);
+ return n;
+ }
if (lineoff && last)
printline(line, lineoff, addr - lineoff);
if (last)
printline((unsigned char *)"", 0, addr);
+ return 0;
}

static int
@@ -196,7 +206,7 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE *fp;
+ int fd;
struct type *t;
int ret = 0, len;
char *s;
@@ -293,25 +303,26 @@ main(int argc, char *argv[])
linelen *= 2;

if (!argc) {
- od(stdin, "<stdin>", 1);
+ if (od(0, "<stdin>", 1) < 0)
+ ret = 1;
} else {
for (; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- od(fp, *argv, (!*(argv + 1)));
- if (fp != stdin && fshut(fp, *argv))
+ if (od(fd, *argv, (!*(argv + 1))) < 0)
ret = 1;
+ if (fd != 0)
+ close(fd);
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>") |
- fshut(stderr, "<stderr>");
+ ret |= fshut(stdout, "<stdout>") | fshut(stderr, "<stderr>");

return ret;
}
diff --git a/tee.c b/tee.c
index 35e3db5..eac106c 100644
--- a/tee.c
+++ b/tee.c
@@ -1,6 +1,7 @@
/* See LICENSE file for copyright and license details. */
+#include <fcntl.h>
#include <signal.h>
-#include <stdio.h>
+#include <unistd.h>

#include "util.h"

@@ -13,14 +14,15 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE **fps = NULL;
- size_t i, n, nfps;
- int ret = 0, aflag = 0, iflag = 0;
+ int *fds = NULL;
+ size_t i, nfds;
+ ssize_t n;
+ int ret = 0, aflag = O_TRUNC, iflag = 0;
char buf[BUFSIZ];

ARGBEGIN {
case 'a':
- aflag = 1;
+ aflag = O_APPEND;
break;
case 'i':
iflag = 1;
@@ -31,31 +33,28 @@ main(int argc, char *argv[])

if (iflag && signal(SIGINT, SIG_IGN) == SIG_ERR)
eprintf("signal:");
- nfps = argc + 1;
- fps = ecalloc(nfps, sizeof(*fps));
+ nfds = argc + 1;
+ fds = ecalloc(nfds, sizeof(*fds));

for (i = 0; i < argc; i++) {
- if (!(fps[i] = fopen(argv[i], aflag ? "a" : "w"))) {
- weprintf("fopen %s:", argv[i]);
+ if ((fds[i] = open(argv[i], O_WRONLY|O_CREAT|aflag, 0666)) < 0) {
+ weprintf("open %s:", argv[i]);
ret = 1;
}
}
- fps[i] = stdout;
+ fds[i] = 1;

- while ((n = fread(buf, 1, sizeof(buf), stdin))) {
- for (i = 0; i < nfps; i++) {
- if (fps[i] && fwrite(buf, 1, n, fps[i]) != n) {
- fshut(fps[i], (i != argc) ? argv[i] : "<stdout>");
- fps[i] = NULL;
+ while ((n = read(0, buf, sizeof(buf))) > 0) {
+ for (i = 0; i < nfds; i++) {
+ if (fds[i] >= 0 && writeall(fds[i], buf, n) < 0) {
+ weprintf("write %s:", (i != argc) ? argv[i] : "<stdout>");
+ fds[i] = -1;
ret = 1;
}
}
}
-
- ret |= fshut(stdin, "<stdin>");
- for (i = 0; i < nfps; i++)
- if (fps[i])
- ret |= fshut(fps[i], (i != argc) ? argv[i] : "<stdout>");
+ if (n < 0)
+ eprintf("read <stdin>:");

return ret;
}
--
2.11.0
Michael Forney
2017-01-02 01:00:34 UTC
Permalink
If we are just copying data from one file to another, we don't need to
fill a complete buffer, just read a chunk at a time, and write it to the
output.
---
cat.c | 39 +++++------
libutil/concat.c | 24 ++++---
libutil/cp.c | 42 +++++-------
sponge.c | 31 +++++----
tail.c | 199 +++++++++++++++++++++++++++++++++----------------------
text.h | 1 -
util.h | 1 +
xinstall.c | 25 ++++---
8 files changed, 196 insertions(+), 166 deletions(-)

diff --git a/cat.c b/cat.c
index e3741aa..211e8d1 100644
--- a/cat.c
+++ b/cat.c
@@ -1,22 +1,11 @@
/* See LICENSE file for copyright and license details. */
-#include <stdio.h>
+#include <fcntl.h>
#include <string.h>
#include <unistd.h>

-#include "text.h"
#include "util.h"

static void
-uconcat(FILE *fp1, const char *s1, FILE *fp2, const char *s2)
-{
- int c;
-
- setbuf(fp2, NULL);
- while ((c = getc(fp1)) != EOF)
- putc(c, fp2);
-}
-
-static void
usage(void)
{
eprintf("usage: %s [-u] [file ...]\n", argv0);
@@ -25,37 +14,39 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE *fp;
- int ret = 0;
- void (*cat)(FILE *, const char *, FILE *, const char *) = &concat;
+ int fd, ret = 0;

ARGBEGIN {
case 'u':
- cat = &uconcat;
break;
default:
usage();
} ARGEND

if (!argc) {
- cat(stdin, "<stdin>", stdout, "<stdout>");
+ if (concat(0, "<stdin>", 1, "<stdout>") < 0)
+ ret = 1;
} else {
for (; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
- cat(fp, *argv, stdout, "<stdout>");
- if (fp != stdin && fshut(fp, *argv))
+ switch (concat(fd, *argv, 1, "<stdout>")) {
+ case -1:
ret = 1;
+ break;
+ case -2:
+ return 1; /* exit on write error */
+ }
+ if (fd != 0)
+ close(fd);
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>");
-
return ret;
}
diff --git a/libutil/concat.c b/libutil/concat.c
index fad9471..2e9aa52 100644
--- a/libutil/concat.c
+++ b/libutil/concat.c
@@ -1,19 +1,23 @@
/* See LICENSE file for copyright and license details. */
-#include <stdio.h>
+#include <unistd.h>

-#include "../text.h"
#include "../util.h"

-void
-concat(FILE *fp1, const char *s1, FILE *fp2, const char *s2)
+int
+concat(int f1, const char *s1, int f2, const char *s2)
{
char buf[BUFSIZ];
- size_t n;
+ ssize_t n;

- while ((n = fread(buf, 1, sizeof(buf), fp1))) {
- fwrite(buf, 1, n, fp2);
-
- if (feof(fp1) || ferror(fp1) || ferror(fp2))
- break;
+ while ((n = read(f1, buf, sizeof(buf))) > 0) {
+ if (writeall(f2, buf, n) < 0) {
+ weprintf("write %s:", s2);
+ return -2;
+ }
+ }
+ if (n < 0) {
+ weprintf("read %s:", s1);
+ return -1;
}
+ return 0;
}
diff --git a/libutil/cp.c b/libutil/cp.c
index 9bb517a..c35d03e 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -12,7 +12,6 @@
#include <utime.h>

#include "../fs.h"
-#include "../text.h"
#include "../util.h"

int cp_aflag = 0;
@@ -27,7 +26,7 @@ int
cp(const char *s1, const char *s2, int depth)
{
DIR *dp;
- FILE *f1, *f2;
+ int f1, f2;
struct dirent *d;
struct stat st;
struct timespec times[2];
@@ -112,43 +111,34 @@ cp(const char *s1, const char *s2, int depth)
return 0;
}
} else {
- if (!(f1 = fopen(s1, "r"))) {
- weprintf("fopen %s:", s1);
+ if ((f1 = open(s1, O_RDONLY)) < 0) {
+ weprintf("open %s:", s1);
cp_status = 1;
return 0;
}
- if (!(f2 = fopen(s2, "w"))) {
- if (cp_fflag) {
- if (unlink(s2) < 0 && errno != ENOENT) {
- weprintf("unlink %s:", s2);
- cp_status = 1;
- return 0;
- } else if (!(f2 = fopen(s2, "w"))) {
- weprintf("fopen %s:", s2);
- cp_status = 1;
- return 0;
- }
- } else {
- weprintf("fopen %s:", s2);
+ if ((f2 = creat(s2, st.st_mode)) < 0 && cp_fflag) {
+ if (unlink(s2) < 0 && errno != ENOENT) {
+ weprintf("unlink %s:", s2);
cp_status = 1;
return 0;
}
+ f2 = creat(s2, st.st_mode);
}
- concat(f1, s1, f2, s2);
-
- /* preserve permissions by default */
- fchmod(fileno(f2), st.st_mode);
-
- if (fclose(f2) == EOF) {
- weprintf("fclose %s:", s2);
+ if (f2 < 0) {
+ weprintf("creat %s:", s2);
cp_status = 1;
return 0;
}
- if (fclose(f1) == EOF) {
- weprintf("fclose %s:", s1);
+ if (concat(f1, s1, f2, s2) < 0) {
cp_status = 1;
return 0;
}
+
+ /* preserve permissions by default */
+ fchmod(f2, st.st_mode);
+
+ close(f1);
+ close(f2);
}

if (cp_aflag || cp_pflag) {
diff --git a/sponge.c b/sponge.c
index baeac7f..da8b28c 100644
--- a/sponge.c
+++ b/sponge.c
@@ -1,7 +1,8 @@
/* See LICENSE file for copyright and license details. */
-#include <stdio.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>

-#include "text.h"
#include "util.h"

static void
@@ -13,24 +14,26 @@ usage(void)
int
main(int argc, char *argv[])
{
- FILE *fp, *tmpfp;
- int ret = 0;
+ char tmp[] = "/tmp/sponge-XXXXXX";
+ int fd, tmpfd;

argv0 = argv[0], argc--, argv++;

if (argc != 1)
usage();

- if (!(tmpfp = tmpfile()))
- eprintf("tmpfile:");
- concat(stdin, "<stdin>", tmpfp, "<tmpfile>");
- rewind(tmpfp);
+ if ((tmpfd = mkstemp(tmp)) < 0)
+ eprintf("mkstemp:");
+ unlink(tmp);
+ if (concat(0, "<stdin>", tmpfd, "<tmpfile>") < 0)
+ return 1;
+ if (lseek(tmpfd, 0, SEEK_SET) < 0)
+ eprintf("lseek:");

- if (!(fp = fopen(argv[0], "w")))
- eprintf("fopen %s:", argv[0]);
- concat(tmpfp, "<tmpfile>", fp, argv[0]);
+ if ((fd = creat(argv[0], 0666)) < 0)
+ eprintf("creat %s:", argv[0]);
+ if (concat(tmpfd, "<tmpfile>", fd, argv[0]) < 0)
+ return 1;

- ret |= fshut(fp, argv[0]) | fshut(tmpfp, "<tmpfile>");
-
- return ret;
+ return 0;
}
diff --git a/tail.c b/tail.c
index 711707f..1ab9d18 100644
--- a/tail.c
+++ b/tail.c
@@ -1,80 +1,125 @@
/* See LICENSE file for copyright and license details. */
#include <sys/stat.h>

+#include <fcntl.h>
+#include <unistd.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

-#include "text.h"
#include "utf.h"
#include "util.h"

static char mode = 'n';

-static void
-dropinit(FILE *fp, const char *str, size_t n)
+static int
+dropinit(int fd, const char *fname, size_t count)
{
Rune r;
- char *buf = NULL;
- size_t size = 0, i = 1;
- ssize_t len;
+ char buf[BUFSIZ], *p;
+ ssize_t n;
+ int nr;
+
+ if (count < 2)
+ goto copy;
+ count--; /* numbering starts at 1 */
+ while (count && (n = read(fd, buf, sizeof(buf))) > 0) {
+ if (mode == 'n') {
+ for (p = buf; count && n > 0; p++, n--) {
+ if (*p == '\n')
+ count--;
+ }
+ } else {
+ for (p = buf; count && n > 0; p += nr, n -= nr, count--) {
+ nr = charntorune(&r, p, n);
+ if (!nr) {
+ /* we don't have a full rune, move
+ * remaining data to beginning and read
+ * again */
+ memmove(buf, p, n);
+ break;
+ }
+ }
+ }
+ }
+ if (count) {
+ if (n < 0)
+ weprintf("read %s:", fname);
+ if (n <= 0)
+ return n;
+ }

- if (mode == 'n') {
- while (i < n && (len = getline(&buf, &size, fp)) > 0)
- if (len > 0 && buf[len - 1] == '\n')
- i++;
- } else {
- while (i < n && efgetrune(&r, fp, str))
- i++;
+ /* write the rest of the buffer */
+ if (writeall(1, p, n) < 0)
+ eprintf("write:");
+copy:
+ switch (concat(fd, fname, 1, "<stdout>")) {
+ case -1: /* read error */
+ return -1;
+ case -2: /* write error */
+ exit(1);
+ default:
+ return 0;
}
- free(buf);
- concat(fp, str, stdout, "<stdout>");
}

-static void
-taketail(FILE *fp, const char *str, size_t n)
+static int
+taketail(int fd, const char *fname, size_t count)
{
- Rune *r = NULL;
- struct line *ring = NULL;
- size_t i, j, *size = NULL;
- ssize_t len;
- int seenln = 0;
-
- if (!n)
- return;
-
- if (mode == 'n') {
- ring = ecalloc(n, sizeof(*ring));
- size = ecalloc(n, sizeof(*size));
-
- for (i = j = 0; (len = getline(&ring[i].data,
- &size[i], fp)) > 0; seenln = 1) {
- ring[i].len = len;
- i = j = (i + 1) % n;
+ static char *buf = NULL;
+ static size_t size = 0;
+ char *p;
+ size_t len = 0, left;
+ ssize_t n;
+
+ if (!count)
+ return 0;
+ for (;;) {
+ if (len + BUFSIZ > size) {
+ /* make sure we have at least BUFSIZ to read */
+ size += 2 * BUFSIZ;
+ buf = erealloc(buf, size);
}
- } else {
- r = ecalloc(n, sizeof(*r));
-
- for (i = j = 0; efgetrune(&r[i], fp, str); )
- i = j = (i + 1) % n;
- }
- if (ferror(fp))
- eprintf("%s: read error:", str);
-
- do {
- if (seenln && ring && ring[j].data) {
- fwrite(ring[j].data, 1, ring[j].len, stdout);
- free(ring[j].data);
- } else if (r) {
- efputrune(&r[j], stdout, "<stdout>");
+ n = read(fd, buf + len, size - len);
+ if (n < 0) {
+ weprintf("read %s:", fname);
+ return -1;
}
- } while ((j = (j + 1) % n) != i);
-
- free(ring);
- free(size);
- free(r);
+ if (n == 0)
+ break;
+ len += n;
+ if (mode == 'n') {
+ /* ignore the last character; if it is a newline, it
+ * ends the last line */
+ for (p = buf + len - 2, left = count; p >= buf; p--) {
+ if (*p != '\n')
+ continue;
+ left--;
+ if (!left) {
+ p++;
+ break;
+ }
+ }
+ } else {
+ for (p = buf + len - 1, left = count; p >= buf; p--) {
+ /* skip utf-8 continuation bytes */
+ if ((*p & 0xc0) == 0x80)
+ continue;
+ left--;
+ if (!left)
+ break;
+ }
+ }
+ if (p > buf) {
+ len -= p - buf;
+ memmove(buf, p, len);
+ }
+ }
+ if (writeall(1, buf, len) < 0)
+ eprintf("write:");
+ return 0;
}

static void
@@ -87,11 +132,11 @@ int
main(int argc, char *argv[])
{
struct stat st1, st2;
- FILE *fp;
- size_t tmpsize, n = 10;
+ int fd;
+ size_t n = 10;
int fflag = 0, ret = 0, newline = 0, many = 0;
- char *numstr, *tmp;
- void (*tail)(FILE *, const char *, size_t) = taketail;
+ char *numstr;
+ int (*tail)(int, const char *, size_t) = taketail;

ARGBEGIN {
case 'f':
@@ -113,17 +158,18 @@ main(int argc, char *argv[])
usage();
} ARGEND

- if (!argc)
- tail(stdin, "<stdin>", n);
- else {
+ if (!argc) {
+ if (tail(0, "<stdin>", n) < 0)
+ ret = 1;
+ } else {
if ((many = argc > 1) && fflag)
usage();
for (newline = 0; *argv; argc--, argv++) {
if (!strcmp(*argv, "-")) {
*argv = "<stdin>";
- fp = stdin;
- } else if (!(fp = fopen(*argv, "r"))) {
- weprintf("fopen %s:", *argv);
+ fd = 0;
+ } else if ((fd = open(*argv, O_RDONLY)) < 0) {
+ weprintf("open %s:", *argv);
ret = 1;
continue;
}
@@ -134,27 +180,26 @@ main(int argc, char *argv[])
if (!(S_ISFIFO(st1.st_mode) || S_ISREG(st1.st_mode)))
fflag = 0;
newline = 1;
- tail(fp, *argv, n);
+ if (tail(fd, *argv, n) < 0) {
+ ret = 1;
+ fflag = 0;
+ }

if (!fflag) {
- if (fp != stdin && fshut(fp, *argv))
- ret = 1;
+ if (fd != 0)
+ close(fd);
continue;
}
- for (tmp = NULL, tmpsize = 0;;) {
- while (getline(&tmp, &tmpsize, fp) > 0) {
- fputs(tmp, stdout);
- fflush(stdout);
- }
- if (ferror(fp))
- eprintf("readline %s:", *argv);
- clearerr(fp);
+ for (;;) {
+ if (concat(fd, *argv, 1, "<stdout>") < 0)
+ exit(1);
/* ignore error in case file was removed, we continue
* tracking the existing open file descriptor */
if (!stat(*argv, &st2)) {
if (st2.st_size < st1.st_size) {
fprintf(stderr, "%s: file truncated\n", *argv);
- rewind(fp);
+ if (lseek(fd, SEEK_SET, 0) < 0)
+ eprintf("lseek:");
}
st1 = st2;
}
@@ -163,7 +208,5 @@ main(int argc, char *argv[])
}
}

- ret |= fshut(stdin, "<stdin>") | fshut(stdout, "<stdout>");
-
return ret;
}
diff --git a/text.h b/text.h
index bceda52..9858592 100644
--- a/text.h
+++ b/text.h
@@ -13,5 +13,4 @@ struct linebuf {
#define EMPTY_LINEBUF {NULL, 0, 0,}
void getlines(FILE *, struct linebuf *);

-void concat(FILE *, const char *, FILE *, const char *);
int linecmp(struct line *, struct line *);
diff --git a/util.h b/util.h
index eaad3ce..968a3af 100644
--- a/util.h
+++ b/util.h
@@ -64,6 +64,7 @@ int eregcomp(regex_t *, const char *, int);

/* io */
ssize_t writeall(int, const void *, size_t);
+int concat(int, const char *, int, const char *);

/* misc */
void enmasse(int, char **, int (*)(const char *, const char *, int));
diff --git a/xinstall.c b/xinstall.c
index d0069be..93ce842 100644
--- a/xinstall.c
+++ b/xinstall.c
@@ -2,6 +2,7 @@
#include <grp.h>
#include <pwd.h>
#include <errno.h>
+#include <fcntl.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
@@ -10,7 +11,6 @@
#include <sys/wait.h>

#include "util.h"
-#include "text.h"

static int Dflag = 0;
static gid_t group;
@@ -44,7 +44,7 @@ static int
install(const char *s1, const char *s2, int depth)
{
DIR *dp;
- FILE *f1, *f2;
+ int f1, f2;
struct dirent *d;
struct stat st;
ssize_t r;
@@ -92,23 +92,22 @@ install(const char *s1, const char *s2, int depth)
else if (mknod(s2, (st.st_mode & ~07777) | mode, st.st_rdev) < 0)
eprintf("mknod %s:", s2);
} else {
- if (!(f1 = fopen(s1, "r")))
- eprintf("fopen %s:", s1);
- if (!(f2 = fopen(s2, "w"))) {
+ if ((f1 = open(s1, O_RDONLY)) < 0)
+ eprintf("open %s:", s1);
+ if ((f2 = creat(s2, 0600)) < 0) {
if (unlink(s2) < 0 && errno != ENOENT)
eprintf("unlink %s:", s2);
- else if (!(f2 = fopen(s2, "w")))
- eprintf("fopen %s:", s2);
+ if ((f2 = creat(s2, 0600)) < 0)
+ eprintf("creat %s:", s2);
}
- concat(f1, s1, f2, s2);
+ if (concat(f1, s1, f2, s2) < 0)
+ exit(1);

- if (fchmod(fileno(f2), mode) < 0)
+ if (fchmod(f2, mode) < 0)
eprintf("fchmod %s:", s2);

- if (fclose(f2) == EOF)
- eprintf("fclose %s:", s2);
- if (fclose(f1) == EOF)
- eprintf("fclose %s:", s1);
+ close(f1);
+ close(f2);
}

if (lchown(s2, owner, group) < 0)
--
2.11.0
Michael Forney
2017-01-02 01:00:35 UTC
Permalink
Previously, when the destination file was created with fopen, we needed
to use fchmod to set its permissions.

Now that we pass in the mode to creat, we already get the desired
behavior of creating the file with the same mode as the source file
modified by the user's file creation mask.

This fixes the issue where a directory or special file created with
mkdir/mknod does not end up with the appropriate mode with -p or -a
(since it may have been narrowed by the umask).

This also allows us to clear the SUID and SGID bits from the mode if the
chown fails, as specified by POSIX.
---
libutil/cp.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libutil/cp.c b/libutil/cp.c
index c35d03e..e212a48 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -134,9 +134,6 @@ cp(const char *s1, const char *s2, int depth)
return 0;
}

- /* preserve permissions by default */
- fchmod(f2, st.st_mode);
-
close(f1);
close(f2);
}
@@ -150,12 +147,16 @@ cp(const char *s1, const char *s2, int depth)
cp_status = 1;
}

- /* owner */
+ /* owner and mode */
if (!S_ISLNK(st.st_mode)) {
if (chown(s2, st.st_uid, st.st_gid) < 0) {
weprintf("chown %s:", s2);
cp_status = 1;
- return 0;
+ st.st_mode &= ~(S_ISUID | S_ISGID);
+ }
+ if (chmod(s2, st.st_mode) < 0) {
+ weprintf("chmod %s:", s2);
+ cp_status = 1;
}
} else {
if (lchown(s2, st.st_uid, st.st_gid) < 0) {
--
2.11.0
Michael Forney
2017-01-02 01:00:36 UTC
Permalink
---
tail.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/tail.c b/tail.c
index 1ab9d18..ce65a01 100644
--- a/tail.c
+++ b/tail.c
@@ -175,8 +175,8 @@ main(int argc, char *argv[])
}
if (many)
printf("%s==> %s <==\n", newline ? "\n" : "", *argv);
- if (stat(*argv, &st1) < 0)
- eprintf("stat %s:", *argv);
+ if (fstat(fd, &st1) < 0)
+ eprintf("fstat %s:", *argv);
if (!(S_ISFIFO(st1.st_mode) || S_ISREG(st1.st_mode)))
fflag = 0;
newline = 1;
@@ -193,16 +193,14 @@ main(int argc, char *argv[])
for (;;) {
if (concat(fd, *argv, 1, "<stdout>") < 0)
exit(1);
- /* ignore error in case file was removed, we continue
- * tracking the existing open file descriptor */
- if (!stat(*argv, &st2)) {
- if (st2.st_size < st1.st_size) {
- fprintf(stderr, "%s: file truncated\n", *argv);
- if (lseek(fd, SEEK_SET, 0) < 0)
- eprintf("lseek:");
- }
- st1 = st2;
+ if (fstat(fd, &st2) < 0)
+ eprintf("fstat %s:", *argv);
+ if (st2.st_size < st1.st_size) {
+ fprintf(stderr, "%s: file truncated\n", *argv);
+ if (lseek(fd, SEEK_SET, 0) < 0)
+ eprintf("lseek:");
}
+ st1 = st2;
sleep(1);
}
}
--
2.11.0
Michael Forney
2017-01-02 01:00:37 UTC
Permalink
POSIX says that -c specifies a number of bytes, not characters. This
flag is commonly used by scripts that operate on binary files to things
like extract a header. Treating the offsets as character offsets will
break things in mysterious ways.

Instead, add a -m option (chosen to match `wc -m`, which also operates
on characters) to handle character offsets.
---
tail.1 | 6 +++---
tail.c | 29 ++++++++++++++++++++++++-----
2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/tail.1 b/tail.1
index 433404d..281560d 100644
--- a/tail.1
+++ b/tail.1
@@ -7,7 +7,7 @@
.Sh SYNOPSIS
.Nm
.Op Fl f
-.Op Fl c Ar num | Fl n Ar num | Fl Ns Ar num
+.Op Fl c Ar num | Fl m Ar num | Fl n Ar num | Fl Ns Ar num
.Op Ar file ...
.Sh DESCRIPTION
.Nm
@@ -20,10 +20,10 @@ is given,
reads from stdin.
.Sh OPTIONS
.Bl -tag -width Ds
-.It Fl c Ar num | Fl n Ar num | Fl Ns Ar num
+.It Fl c Ar num | Fl m Ar num | Fl n Ar num | Fl Ns Ar num
Display final
.Ar num
-characters | lines |
+bytes | characters | lines |
lines. If
.Ar num
begins with '+'
diff --git a/tail.c b/tail.c
index ce65a01..ce3be9d 100644
--- a/tail.c
+++ b/tail.c
@@ -26,12 +26,23 @@ dropinit(int fd, const char *fname, size_t count)
goto copy;
count--; /* numbering starts at 1 */
while (count && (n = read(fd, buf, sizeof(buf))) > 0) {
- if (mode == 'n') {
+ switch (mode) {
+ case 'n': /* lines */
for (p = buf; count && n > 0; p++, n--) {
if (*p == '\n')
count--;
}
- } else {
+ break;
+ case 'c': /* bytes */
+ if (count > n) {
+ count -= n;
+ } else {
+ p = buf + count;
+ n -= count;
+ count = 0;
+ }
+ break;
+ case 'm': /* runes */
for (p = buf; count && n > 0; p += nr, n -= nr, count--) {
nr = charntorune(&r, p, n);
if (!nr) {
@@ -42,6 +53,7 @@ dropinit(int fd, const char *fname, size_t count)
break;
}
}
+ break;
}
}
if (count) {
@@ -90,7 +102,8 @@ taketail(int fd, const char *fname, size_t count)
if (n == 0)
break;
len += n;
- if (mode == 'n') {
+ switch (mode) {
+ case 'n': /* lines */
/* ignore the last character; if it is a newline, it
* ends the last line */
for (p = buf + len - 2, left = count; p >= buf; p--) {
@@ -102,7 +115,11 @@ taketail(int fd, const char *fname, size_t count)
break;
}
}
- } else {
+ break;
+ case 'c': /* bytes */
+ p = count < len ? buf + len - count : buf;
+ break;
+ case 'm': /* runes */
for (p = buf + len - 1, left = count; p >= buf; p--) {
/* skip utf-8 continuation bytes */
if ((*p & 0xc0) == 0x80)
@@ -111,6 +128,7 @@ taketail(int fd, const char *fname, size_t count)
if (!left)
break;
}
+ break;
}
if (p > buf) {
len -= p - buf;
@@ -125,7 +143,7 @@ taketail(int fd, const char *fname, size_t count)
static void
usage(void)
{
- eprintf("usage: %s [-f] [-c num | -n num | -num] [file ...]\n", argv0);
+ eprintf("usage: %s [-f] [-c num | -m num | -n num | -num] [file ...]\n", argv0);
}

int
@@ -143,6 +161,7 @@ main(int argc, char *argv[])
fflag = 1;
break;
case 'c':
+ case 'm':
case 'n':
mode = ARGC();
numstr = EARGF(usage());
--
2.11.0
Loading...