Jeremie Courreges-Anglas wrote:
> Today someone bumped into a port that contained head -c calls. While
> upstream could be prodded to care a bit more about portability, support
> for head -c is widespread (GNU coreutils, busybox, FreeBSD, NetBSD) so
> I don't see any value in being different. Plus, tail(1) already has
> support for -c.
>
> Comments/ok?
Makes sense and works for me. I'll leave a few comments inline. Also:
> PS: the next diff will remove documentation for the obsolete "-count"
> syntax.
In what sense is it obsolete? I've always used this and I think it's
pretty standard these days. Now that I look, it seems that GNU head(1)
has it but FreeBSD's and NetBSD's don't. I'd be sad to see it go.
> Index: head.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/head/head.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 head.c
> --- head.c 9 Oct 2015 01:37:07 -0000 1.20
> +++ head.c 9 Mar 2016 20:02:08 -0000
> @@ -51,9 +51,9 @@ main(int argc, char *argv[])
> FILE *fp;
> long cnt;
> int ch, firsttime;
> - long linecnt = 10;
> + long count = 10;
It seems like it'd be better to make this a long long for the sake of
robustness. It's conceivable that someone could try to truncate a file
to a few gigabytes.
> char *p = NULL;
> - int status = 0;
> + int dobytes = 0, status = 0;
I'd definitely prefer stdbool here, but that's contentious.
>
> if (pledge("stdio rpath", NULL) == -1)
> err(1, "pledge");
> @@ -66,13 +66,18 @@ main(int argc, char *argv[])
> argv++;
> }
>
> - while ((ch = getopt(argc, argv, "n:")) != -1) {
> + while ((ch = getopt(argc, argv, "c:n:")) != -1) {
> switch (ch) {
> + case 'c':
> + dobytes = 1;
> + p = optarg;
> + break;
> case 'n':
> + dobytes = 0;
It's already initialized to 0, which is necessary for the head -count
style syntax.
> p = optarg;
> break;
> default:
> - usage();
> + usage();
Good catch.
> }
> }
> argc -= optind, argv += optind;
> @@ -80,9 +85,10 @@ main(int argc, char *argv[])
> if (p) {
> const char *errstr;
>
> - linecnt = strtonum(p, 1, LONG_MAX, &errstr);
> + count = strtonum(p, 1, LONG_MAX, &errstr);
If using long long, this would have to change too.
> if (errstr)
> - errx(1, "line count %s: %s", errstr, p);
> + errx(1, "%s count %s: %s", dobytes ? "bytes" : "lines",
> + errstr, p);
> }
>
> for (firsttime = 1; ; firsttime = 0) {
> @@ -105,10 +111,16 @@ main(int argc, char *argv[])
> }
> ++argv;
> }
> - for (cnt = linecnt; cnt && !feof(fp); --cnt)
> - while ((ch = getc(fp)) != EOF)
> - if (putchar(ch) == '\n')
> - break;
> + if (dobytes) {
> + for (cnt = count; cnt && !feof(fp); --cnt)
> + if ((ch = getc(fp)) != EOF)
> + putchar(ch);
> + } else {
> + for (cnt = count; cnt && !feof(fp); --cnt)
> + while ((ch = getc(fp)) != EOF)
> + if (putchar(ch) == '\n')
> + break;
> + }
Total nitpick, but I think switching the for loop condition to "cnt > 0
&& !feof(fp)" would be nice.
> fclose(fp);
> }
> /*NOTREACHED*/
Might as well delete /*NOTREACHED*/
