Hi,
On Tue, Aug 14 2018 12:08:54 -0500, Scott Cheloha wrote:
> > + len = (size_t)linelen;
> > + if (line[len - 1] == '\n')
> > + line[len - 1] = '\0';
>
> You can just use linelen directly and skip the assignment.
I did it this way because this bit later
if (base64)
cmp = strncmp(checksum, digest, len);
could have been reached without assigning len (if the "could be GNU form"
branch is taken). It looks like checksum and digest will always be null
temrinated, but 'len' may be reduced to something less than linelen
(eg. in the case that the line starts with whitespace). Maybe there's
some reason to not use strcmp here?
> > + while (isspace((unsigned char)*line))
> > + line++;
>
> Incrementing the returned pointer works for fgetln(3) because it
> keeps track of its own buffer internally. But if you try the same
> trick with getline(3) it may try to realloc your invalid pointer.
Ah, of course. Stupid mistake. Fixed below.
> > @@ -725,11 +720,11 @@ digest_filelist(const char *file, struct
> > hash_function *defhash, int selcount,
> > error = 1;
> > }
> > }
> > + free(line);
>
> getline(3) can fail, too, so you need to check listfp for errors after
> freeing line.
So can fgetln(3), but fair enough.
But shouldn't we check *before* calling free to ensure errno is not
clobbered? (although, getline.3's example code certainly does err(1,
"getline"); after free()...)
diff --git a/bin/md5/md5.c b/bin/md5/md5.c
index 84adbcf0989..11345bcaa2a 100644
--- a/bin/md5/md5.c
+++ b/bin/md5/md5.c
@@ -549,11 +549,11 @@ digest_filelist(const char *file, struct hash_function
*defhash, int selcount,
int found, base64, error, cmp, i;
size_t algorithm_max, algorithm_min;
const char *algorithm;
- char *filename, *checksum, *buf, *p;
+ char *filename, *checksum, *line, *p;
char digest[MAX_DIGEST_LEN + 1];
- char *lbuf = NULL;
+ ssize_t linelen;
FILE *listfp, *fp;
- size_t len, nread;
+ size_t len, linesize, nread;
int *sel_found = NULL;
u_char data[32 * 1024];
union ANY_CTX context;
@@ -580,20 +580,16 @@ digest_filelist(const char *file, struct hash_function
*defhash, int selcount,
}
error = found = 0;
- while ((buf = fgetln(listfp, &len))) {
+ line = NULL;
+ linesize = 0;
+ while ((linelen = getline(&line, &linesize, listfp)) != -1) {
+ char *tmpline = line;
base64 = 0;
- if (buf[len - 1] == '\n')
- buf[len - 1] = '\0';
- else {
- if ((lbuf = malloc(len + 1)) == NULL)
- err(1, NULL);
-
- (void)memcpy(lbuf, buf, len);
- lbuf[len] = '\0';
- buf = lbuf;
- }
- while (isspace((unsigned char)*buf))
- buf++;
+ len = (size_t)linelen;
+ if (line[len - 1] == '\n')
+ line[len - 1] = '\0';
+ while (isspace((unsigned char)*tmpline))
+ tmpline++;
/*
* Crack the line into an algorithm, filename, and checksum.
@@ -603,11 +599,11 @@ digest_filelist(const char *file, struct hash_function
*defhash, int selcount,
* Fallback on GNU form:
* CHECKSUM FILENAME
*/
- p = strchr(buf, ' ');
+ p = strchr(tmpline, ' ');
if (p != NULL && *(p + 1) == '(') {
/* BSD form */
*p = '\0';
- algorithm = buf;
+ algorithm = tmpline;
len = strlen(algorithm);
if (len > algorithm_max || len < algorithm_min)
continue;
@@ -658,7 +654,7 @@ digest_filelist(const char *file, struct hash_function
*defhash, int selcount,
if ((hf = defhash) == NULL)
continue;
algorithm = hf->name;
- checksum = buf;
+ checksum = tmpline;
if ((p = strchr(checksum, ' ')) == NULL)
continue;
if (hf->style == STYLE_CKSUM) {
@@ -725,11 +721,13 @@ digest_filelist(const char *file, struct hash_function
*defhash, int selcount,
error = 1;
}
}
+ if (ferror(listfp))
+ warn("%s: getline", file);
+ free(line);
if (listfp != stdin)
fclose(listfp);
if (!found)
warnx("%s: no properly formatted checksum lines found", file);
- free(lbuf);
if (sel_found != NULL) {
/*
* Mark found files by setting them to NULL so that we can
--
Lauri Tirkkonen | lotheac @ IRCnet
