Michael McConville <[email protected]> writes:
> 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?
1.1 (deraadt 18-Oct-95): /* handle obsolete -number syntax */
1.16 (deraadt 26-Nov-13): if (argc > 1 && argv[1][0] == '-' &&
1.16 (deraadt 26-Nov-13): isdigit((unsigned char)argv[1][1]))
{
1.12 (ray 06-Oct-06): p = argv[1] + 1;
1.13 (tedu 09-Oct-06): argc--;
1.13 (tedu 09-Oct-06): argv++;
1.1 (deraadt 18-Oct-95): }
POSIX 2008 doesn't specify it any more.
> 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.
I'm not saying we should stop supporting it. FreeBSD and NetBSD support
this syntax, they just don't advertise it as prominently.
>> 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.
That's another diff. ;)
>> 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.
I followed the behavior of GNU head here: the last specified option
wins. IIRC FreeBSD head(1) makes -c and -n incompatible.
>> 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.
ok for this one
>> fclose(fp);
>> }
>> /*NOTREACHED*/
>
> Might as well delete /*NOTREACHED*/
and this one if you want to deal with them.
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
