Alex Karle:
> Looking deeper, there is a bug in quiz(6) for datfiles with multi-line
> answers.
>
> Specifically, the following quiz.db line:
>
> foo:\
> bar
>
> Is parsed into "foo:bar\n", which made it impossible to get right (since
> the comparison was expecting a newline). This is a result of a bug in
> quiz.c's appdstr(qp->q_text, lp, len), which copies the trailing newline
> after "bar" (in lp) into q_text.
[...]
> That said, it seemed cleaner to modify the code to use getline(3) over
> fgetln(3), which guarantees each iteration of the loop is a single line
> and null-terminated, enabling the newline truncation in the loop and
> allowing the use of strlcpy(3). This patch implements that fix.
>
> One can verify the bug(fix) by:
>
> $ cat <<EOF > /tmp/qtest
> foo1:bar
> foo2:\\
> bar
> foo3:\\
> ba\\
> r
> EOF
> $ echo "/tmp/qtest:test:lines" > /tmp/qindex
>
> # answer 'bar' to each question
> $ quiz -i /tmp/qindex test lines # fails
> $ /usr/obj/games/quiz/quiz -i /tmp/qindex test lines # succeeds
Thanks a lot for figuring this out! I finally got around to looking
at your patch. Once we have nul-terminated lines, appdstr() can
be replaced with realloc() and strlcat().
This could use another pair of eyes.
OK?
Index: quiz.c
===================================================================
RCS file: /cvs/src/games/quiz/quiz.c,v
retrieving revision 1.30
diff -u -p -r1.30 quiz.c
--- quiz.c 24 Aug 2018 11:14:49 -0000 1.30
+++ quiz.c 9 Mar 2021 20:38:20 -0000
@@ -48,7 +48,6 @@ static QE qlist;
static int catone, cattwo, tflag;
static u_int qsize;
-char *appdstr(char *, const char *, size_t);
void downcase(char *);
void get_cats(char *, char *);
void get_file(const char *);
@@ -110,7 +109,8 @@ get_file(const char *file)
{
FILE *fp;
QE *qp;
- size_t len;
+ ssize_t len;
+ size_t qlen, size;
char *lp;
if ((fp = fopen(file, "r")) == NULL)
@@ -123,26 +123,35 @@ get_file(const char *file)
*/
qp = &qlist;
qsize = 0;
- while ((lp = fgetln(fp, &len)) != NULL) {
+ qlen = 0;
+ lp = NULL;
+ size = 0;
+ while ((len = getline(&lp, &size, fp)) != -1) {
if (lp[len - 1] == '\n')
- --len;
+ lp[--len] = '\0';
if (qp->q_text && qp->q_text[0] != '\0' &&
- qp->q_text[strlen(qp->q_text) - 1] == '\\')
- qp->q_text = appdstr(qp->q_text, lp, len);
- else {
+ qp->q_text[qlen - 1] == '\\') {
+ qp->q_text[--qlen] = '\0';
+ qp->q_text = realloc(qp->q_text, qlen + len + 1);
+ if (qp->q_text == NULL)
+ errx(1, "realloc");
+ qlen = strlcat(qp->q_text, lp, qlen + len + 1);
+ } else {
if ((qp->q_next = malloc(sizeof(QE))) == NULL)
errx(1, "malloc");
qp = qp->q_next;
- if ((qp->q_text = malloc(len + 1)) == NULL)
- errx(1, "malloc");
- /* lp may not be zero-terminated; cannot use strlcpy */
- strncpy(qp->q_text, lp, len);
- qp->q_text[len] = '\0';
+ qp->q_text = strdup(lp);
+ if (qp->q_text == NULL)
+ errx(1, "strdup");
+ qlen = strlen(qp->q_text);
qp->q_asked = qp->q_answered = FALSE;
qp->q_next = NULL;
++qsize;
}
}
+ free(lp);
+ if (ferror(fp))
+ err(1, "getline");
(void)fclose(fp);
}
@@ -316,32 +325,6 @@ next_cat(const char *s)
esc = 0;
break;
}
-}
-
-char *
-appdstr(char *s, const char *tp, size_t len)
-{
- char *mp;
- const char *sp;
- int ch;
- char *m;
-
- if ((m = malloc(strlen(s) + len + 1)) == NULL)
- errx(1, "malloc");
- for (mp = m, sp = s; (*mp++ = *sp++) != '\0'; )
- ;
- --mp;
- if (*(mp - 1) == '\\')
- --mp;
-
- while ((ch = *mp++ = *tp++) && ch != '\n')
- ;
- if (*(mp - 2) == '\\')
- mp--;
- *mp = '\0';
-
- free(s);
- return (m);
}
void
--
Christian "naddy" Weisgerber [email protected]
