Hi,
Lauri Tirkkonen wrote on Tue, Jul 17, 2018 at 09:14:29PM +0300:
> In the same vein as my previous diff for join(1),
> make paste(1) use getline instead of fgetln.
I think it's correct, but i tweaked it a bit for simplicity.
In sequential(), the double loop with double getline(), both with
error handling etc. etc. really felt too ugly to me, so i reduced
it to one simple loop with one call to getline().
In parallel(), tweaks are minimal:
* sorted declarations
* print the filename in case of I/O errors, unless it's stdin
* simplify printf(3) to fputs(3)
So i'd like to commit the following version.
OK?
Ingo
P.S.
Final diff is -44 +31 (-5% of the whole program).
I love make stuff better and smaller at the same time.
Index: paste.c
===================================================================
RCS file: /cvs/src/usr.bin/paste/paste.c,v
retrieving revision 1.23
diff -u -p -r1.23 paste.c
--- paste.c 2 Jan 2018 06:56:41 -0000 1.23
+++ paste.c 29 Jul 2018 15:21:02 -0000
@@ -104,11 +104,11 @@ parallel(char **argv)
{
SIMPLEQ_HEAD(, list) head = SIMPLEQ_HEAD_INITIALIZER(head);
struct list *lp;
+ char *line, *p;
+ size_t len, linesize;
int cnt;
- char ch, *p;
int opencnt, output;
- char *buf, *lbuf;
- size_t len;
+ char ch;
for (cnt = 0; (p = *argv); ++argv, ++cnt) {
if (!(lp = malloc(sizeof(struct list))))
@@ -123,17 +123,22 @@ parallel(char **argv)
SIMPLEQ_INSERT_TAIL(&head, lp, entries);
}
+ line = NULL;
+ linesize = 0;
+
for (opencnt = cnt; opencnt;) {
output = 0;
SIMPLEQ_FOREACH(lp, &head, entries) {
- lbuf = NULL;
if (!lp->fp) {
if (output && lp->cnt &&
(ch = delim[(lp->cnt - 1) % delimcnt]))
putchar(ch);
continue;
}
- if (!(buf = fgetln(lp->fp, &len))) {
+ if ((len = getline(&line, &linesize, lp->fp)) == -1) {
+ if (ferror(lp->fp))
+ err(1, "%s", lp->fp == stdin ?
+ "getline" : lp->name);
if (!--opencnt)
break;
if (lp->fp != stdin)
@@ -144,15 +149,8 @@ parallel(char **argv)
putchar(ch);
continue;
}
- if (buf[len - 1] == '\n')
- buf[len - 1] = '\0';
- else {
- if ((lbuf = malloc(len + 1)) == NULL)
- err(1, "malloc");
- memcpy(lbuf, buf, len);
- lbuf[len] = '\0';
- buf = lbuf;
- }
+ if (line[len - 1] == '\n')
+ line[len - 1] = '\0';
/*
* make sure that we don't print any delimiters
* unless there's a non-empty file.
@@ -164,59 +162,49 @@ parallel(char **argv)
putchar(ch);
} else if ((ch = delim[(lp->cnt - 1) % delimcnt]))
putchar(ch);
- (void)printf("%s", buf);
- if (lbuf)
- free(lbuf);
+ fputs(line, stdout);
}
if (output)
putchar('\n');
}
+ free(line);
}
void
sequential(char **argv)
{
FILE *fp;
+ char *line, *p;
+ size_t len, linesize;
int cnt;
- char ch, *p, *dp;
- char *buf, *lbuf;
- size_t len;
+ line = NULL;
+ linesize = 0;
for (; (p = *argv); ++argv) {
- lbuf = NULL;
if (p[0] == '-' && !p[1])
fp = stdin;
else if (!(fp = fopen(p, "r"))) {
warn("%s", p);
continue;
}
- if ((buf = fgetln(fp, &len))) {
- for (cnt = 0, dp = delim;;) {
- if (buf[len - 1] == '\n')
- buf[len - 1] = '\0';
- else {
- if ((lbuf = malloc(len + 1)) == NULL)
- err(1, "malloc");
- memcpy(lbuf, buf, len);
- lbuf[len] = '\0';
- buf = lbuf;
- }
- (void)printf("%s", buf);
- if (!(buf = fgetln(fp, &len)))
- break;
- if ((ch = *dp++))
- putchar(ch);
- if (++cnt == delimcnt) {
- dp = delim;
- cnt = 0;
- }
- }
- putchar('\n');
+ cnt = -1;
+ while ((len = getline(&line, &linesize, fp)) != -1) {
+ if (line[len - 1] == '\n')
+ line[len - 1] = '\0';
+ if (cnt >= 0)
+ putchar(delim[cnt]);
+ if (++cnt == delimcnt)
+ cnt = 0;
+ fputs(line, stdout);
}
+ if (ferror(fp))
+ err(1, "%s", fp == stdin ? "getline" : p);
+ if (cnt >= 0)
+ putchar('\n');
if (fp != stdin)
(void)fclose(fp);
- free(lbuf);
}
+ free(line);
}
int