Ben Fuller <ben@bvnf.space> wrote:
> Did anyone have a look at this? Feedback is appreciated.
>
> On Sun, Jul 03, 2022 at 02:31:42 +0100, Ben Fuller wrote:
> > One of the fgetln(3)s was previously converted to getline(3), but not
> > the other.
> >
> > diff --git games/quiz/quiz.c games/quiz/quiz.c
> > index d4fd0604e0d..362fb1e24a8 100644
> > --- games/quiz/quiz.c
> > +++ games/quiz/quiz.c
> > @@ -224,7 +224,8 @@ quiz(void)
> > {
> > QE *qp;
> > int i;
> > - size_t len;
> > + size_t size;
> > + ssize_t len;
> > u_int guesses, rights, wrongs;
> > int next;
> > char *answer, *t, question[LINE_SZ];
> > @@ -275,12 +276,15 @@ quiz(void)
> > qp->q_answered = TRUE;
> > continue;
> > }
> > + size = 0;
> > + answer = NULL;
here you're leaking memory.
This whole block is in a loop so you're possibly overriding the memory
allocated in a previous cycle. size and answer should be initialized
outside of the loop.
otherwise the diff reads fine to me and game works fine.
> > qp->q_asked = TRUE;
> > (void)printf("%s?\n", question);
> > for (;; ++guesses) {
> > - if ((answer = fgetln(stdin, &len)) == NULL ||
> > + if ((len = getline(&answer, &size, stdin)) == -1 ||
> > answer[len - 1] != '\n') {
> > score(rights, wrongs, guesses);
> > + free(answer);
> > exit(0);
> > [...]
however, while limiting ourselves to replace fgetln with getline in
quiz(6) only? other games could take advantage of that :)
(i couldn't help myself not to tweak save_file_name in battlestar too)
briefly (play)tested the affected games, everything seems fine.
ok?
diff /usr/src
commit - faf550173e173cd2ef8642601dc48202a09fd44f
path + /usr/src
blob - f7b08498dfdda4b4021212344862399e3c71728b
file + games/battlestar/cypher.c
--- games/battlestar/cypher.c
+++ games/battlestar/cypher.c
@@ -84,8 +84,9 @@ cypher(void)
int n;
int junk;
int lflag = -1;
- char *filename, *rfilename;
- size_t filename_len;
+ char *line = NULL, *filename;
+ size_t linesize = 0;
+ ssize_t linelen;
while (wordnumber <= wordcount) {
if (wordtype[wordnumber] != VERB &&
@@ -357,19 +358,16 @@ cypher(void)
case SAVE:
printf("\nSave file name (default %s): ",
DEFAULT_SAVE_FILE);
- filename = fgetln(stdin, &filename_len);
- if (filename_len == 0
- || (filename_len == 1 && filename[0] == '\n'))
- rfilename = save_file_name(DEFAULT_SAVE_FILE,
- strlen(DEFAULT_SAVE_FILE));
+ linelen = getline(&line, &linesize, stdin);
+ if (linelen <= 1)
+ filename = save_file_name(DEFAULT_SAVE_FILE);
else {
- if (filename[filename_len - 1] == '\n')
- filename_len--;
- rfilename = save_file_name(filename,
- filename_len);
+ if (line[linelen - 1] == '\n')
+ line[linelen - 1] = '\0';
+ filename = save_file_name(line);
}
- save(rfilename);
- free(rfilename);
+ save(filename);
+ free(filename);
break;
case VERBOSE:
@@ -463,6 +461,9 @@ cypher(void)
}
if (!lflag)
newlocation();
+
+ free(line);
+
if (wordnumber < wordcount && !stop_cypher &&
(*words[wordnumber] == ',' || *words[wordnumber] == '.')) {
wordnumber++;
blob - 21f8dc722600d3e169260e6e885319864c856807
file + games/battlestar/extern.h
--- games/battlestar/extern.h
+++ games/battlestar/extern.h
@@ -357,7 +357,7 @@ void ravage(void);
void restore(const char *);
int ride(void);
void save(const char *);
-char *save_file_name(const char *, size_t);
+char *save_file_name(const char *);
int shoot(void);
int take(unsigned int[]);
int takeoff(void);
blob - 5c2044a918e667bc505d35a2f27718d4dad0ba4a
file + games/battlestar/init.c
--- games/battlestar/init.c
+++ games/battlestar/init.c
@@ -66,7 +66,7 @@ initialize(const char *filename)
for (p = dayobjs; p->room != 0; p++)
SetBit(location[p->room].objects, p->obj);
} else {
- savefile = save_file_name(filename, strlen(filename));
+ savefile = save_file_name(filename);
restore(savefile);
free(savefile);
}
blob - 088bb2d2c5cb0f0cfff6485110646301c1665845
file + games/battlestar/save.c
--- games/battlestar/save.c
+++ games/battlestar/save.c
@@ -145,44 +145,26 @@ save(const char *filename)
}
/*
- * Given a save file name (possibly from fgetln, so without terminating NUL),
- * determine the name of the file to be saved to by adding the HOME
- * directory if the name does not contain a slash. Name will be allocated
- * with malloc(3).
+ * Given a save file name determine the name of the file to be saved
+ * to by adding the HOME directory if the name does not contain a
+ * slash. Name will be allocated with malloc(3).
*/
char *
-save_file_name(const char *filename, size_t len)
+save_file_name(const char *filename)
{
char *home;
char *newname;
- size_t tmpl;
- if (memchr(filename, '/', len)) {
- if ((newname = malloc(len + 1)) == NULL) {
+ home = getenv("HOME");
+ if (strchr(filename, '/') != NULL || home == NULL) {
+ if ((newname = strdup(filename)) == NULL)
warnx("out of memory");
- return NULL;
- }
- memcpy(newname, filename, len);
- newname[len] = 0;
- } else {
- if ((home = getenv("HOME")) != NULL) {
- tmpl = strlen(home);
- if ((newname = malloc(tmpl + len + 2)) == NULL) {
- warnx("out of memory");
- return NULL;
- }
- memcpy(newname, home, tmpl);
- newname[tmpl] = '/';
- memcpy(newname + tmpl + 1, filename, len);
- newname[tmpl + len + 1] = 0;
- } else {
- if ((newname = malloc(len + 1)) == NULL) {
- warnx("out of memory");
- return NULL;
- }
- memcpy(newname, filename, len);
- newname[len] = 0;
- }
+ return(newname);
}
+
+ if (asprintf(&newname, "%s/%s", home, filename) == -1) {
+ warnx("out of memory");
+ return NULL;
+ }
return(newname);
}
blob - 8c36bb1f110f2a2e0150e791e0917e8800c19483
file + games/hunt/huntd/conf.c
--- games/hunt/huntd/conf.c
+++ games/hunt/huntd/conf.c
@@ -247,24 +247,19 @@ parse_line(char *buf, char *fnm, int *line)
static void
load_config(FILE *f, char *fnm)
{
- char buf[BUFSIZ];
- size_t len;
- int line;
- char *p;
+ int lineno;
+ char *line = NULL;
+ size_t linesize = 0;
+ ssize_t linelen;
- line = 0;
- while ((p = fgetln(f, &len)) != NULL) {
- line++;
- if (p[len-1] == '\n')
- len--;
- if (len >= sizeof(buf)) {
- logx(LOG_ERR, "%s:%d: line too long", fnm, line);
- continue;
- }
- (void)memcpy(buf, p, len);
- buf[len] = '\0';
- parse_line(buf, fnm, &line);
+ lineno = 0;
+ while ((linelen = getline(&line, &linesize, stdin)) != -1) {
+ lineno++;
+ if (linelen > 1 && line[linelen - 1] == '\n')
+ line[linelen - 1] = '\0';
+ parse_line(line, fnm, &lineno);
}
+ free(line);
}
/*
blob - e8804e1a23d8d5c5e639f8ea59a09592097bf344
file + games/monop/execute.c
--- games/monop/execute.c
+++ games/monop/execute.c
@@ -285,7 +285,8 @@ rest_f(char *file)
int i, j, num;
FILE *inf;
char *st, *a, *b;
- size_t len;
+ size_t linesize;
+ ssize_t len;
STAT sbuf;
int t1;
short t2, t3, t4;
@@ -302,19 +303,22 @@ rest_f(char *file)
}
num = 1;
- st = fgetln(inf, &len);
- if (st == NULL || len != strlen(MONOP_TAG) + 1 ||
+ st = NULL;
+ linesize = 0;
+ len = getline(&st, &linesize, inf);
+ if (len == -1 || len != strlen(MONOP_TAG) + 1 ||
strncmp(st, MONOP_TAG, strlen(MONOP_TAG))) {
badness:
warnx("%s line %d", file, num);
+ free(st);
fclose(inf);
return(FALSE);
}
num++;
- if (fgetln(inf, &len) == NULL)
+ if ((len = getline(&st, &linesize, inf)) == -1)
goto badness;
num++;
- if ((st = fgetln(inf, &len)) == NULL || st[len - 1] != '\n')
+ if ((len = getline(&st, &linesize, inf)) == -1 || st[len - 1] != '\n')
goto badness;
st[len - 1] = '\0';
if (sscanf(st, "%d %d %d", &num_play, &player, &num_doub) != 3 ||
@@ -328,7 +332,8 @@ badness:
/* Names */
for (i = 0; i < num_play; i++) {
num++;
- if ((st = fgetln(inf, &len)) == NULL || st[len - 1] != '\n')
+ if ((len = getline(&st, &linesize, inf)) == -1 ||
+ st[len - 1] != '\n')
goto badness;
st[len - 1] = '\0';
if ((name_list[i] = play[i].name = strdup(st)) == NULL)
@@ -340,7 +345,8 @@ badness:
/* Money, location, GOJF cards, turns in jail */
for (i = 0; i < num_play; i++) {
num++;
- if ((st = fgetln(inf, &len)) == NULL || st[len - 1] != '\n')
+ if ((len = getline(&st, &linesize, inf)) == -1 ||
+ st[len - 1] != '\n')
goto badness;
st[len - 1] = '\0';
if (sscanf(st, "%d %hd %hd %hd", &(play[i].money), &t2,
@@ -355,7 +361,8 @@ badness:
/* Deck status; init_decks() must have been called. */
for (i = 0; i < 2; i++) {
num++;
- if ((st = fgetln(inf, &len)) == NULL || st[len - 1] != '\n')
+ if ((len = getline(&st, &linesize, inf)) == -1 ||
+ st[len - 1] != '\n')
goto badness;
st[len - 1] = '\0';
if (sscanf(st, "%d %d %hd", &t1, &j, &t2) != 3 ||
@@ -365,7 +372,8 @@ badness:
deck[i].top_card = j;
deck[i].gojf_used = t2;
num++;
- if ((st = fgetln(inf, &len)) == NULL || st[len - 1] != '\n')
+ if ((len = getline(&st, &linesize, inf)) == -1 ||
+ st[len - 1] != '\n')
goto badness;
st[len - 1] = '\0';
a = st;
@@ -379,7 +387,7 @@ badness:
/* Ignore anything trailing */
}
trading = FALSE;
- while ((st = fgetln(inf, &len)) != NULL) {
+ while ((len = getline(&st, &linesize, inf)) != -1) {
num++;
if (st[len - 1] != '\n')
goto badness;
@@ -403,6 +411,7 @@ badness:
* within 1 in each monopoly
*/
}
+ free(st);
fclose(inf);
/* Check total hotel and house count */
t1 = j = 0;
blob - d4fd0604e0de22cc4fe3f9e82940e4a449589413
file + games/quiz/quiz.c
--- games/quiz/quiz.c
+++ games/quiz/quiz.c
@@ -224,12 +224,16 @@ quiz(void)
{
QE *qp;
int i;
- size_t len;
+ size_t size;
+ ssize_t len;
u_int guesses, rights, wrongs;
int next;
char *answer, *t, question[LINE_SZ];
const char *s;
+ size = 0;
+ answer = NULL;
+
guesses = rights = wrongs = 0;
for (;;) {
if (qsize == 0)
@@ -278,9 +282,10 @@ quiz(void)
qp->q_asked = TRUE;
(void)printf("%s?\n", question);
for (;; ++guesses) {
- if ((answer = fgetln(stdin, &len)) == NULL ||
+ if ((len = getline(&answer, &size, stdin)) == -1 ||
answer[len - 1] != '\n') {
score(rights, wrongs, guesses);
+ free(answer);
exit(0);
}
answer[len - 1] = '\0';
@@ -302,6 +307,7 @@ quiz(void)
}
}
score(rights, wrongs, guesses);
+ free(answer);
}
const char *
