Hi And thanks for the patches!
Comments below. On Mon, Dec 5, 2016 at 6:55 AM, Michael Forney <[email protected]> wrote: > Previously, if a file failed to read in a checksum list, it would be > reported as not matched rather than a read failure. > > Also, if reading from stdin failed, previously a bogus checksum would be > printed anyway. > --- > libutil/crypt.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/libutil/crypt.c b/libutil/crypt.c > index 3f849ba..6991c39 100644 > --- a/libutil/crypt.c > +++ b/libutil/crypt.c > @@ -64,7 +64,10 @@ mdchecklist(FILE *listfp, struct crypt_ops *ops, uint8_t > *md, size_t sz, > (*noread)++; > continue; > } > - cryptsum(ops, fp, file, md); > + if (cryptsum(ops, fp, file, md)) { > + (*noread)++; > + continue; > + } > r = mdcheckline(line, md, sz); > if (r == 1) { > printf("%s: OK\n", file); > @@ -125,8 +128,10 @@ cryptmain(int argc, char *argv[], struct crypt_ops *ops, > uint8_t *md, size_t sz) > int ret = 0; > > if (argc == 0) { > - cryptsum(ops, stdin, "<stdin>", md); > - mdprint(md, "<stdin>", sz); > + if (cryptsum(ops, stdin, "<stdin>", md)) > + ret = 1; The return values in crypt.c are not consistent which tripped me up here. "cryptsum" returns 1 on error (as does "cryptmain") but "mdcheckline" returns 0 (or -1) on error and 1 on success. For consistency we should change "mdcheckline" to return 1/-1 on error and 0 on success. If we make that change it should be in a different patch though. > + else > + mdprint(md, "<stdin>", sz); > } else { > for (; *argv; argc--, argv++) { > if ((*argv)[0] == '-' && !(*argv)[1]) { > @@ -137,11 +142,10 @@ cryptmain(int argc, char *argv[], struct crypt_ops > *ops, uint8_t *md, size_t sz) > ret = 1; > continue; > } > - if (cryptsum(ops, fp, *argv, md)) { > + if (cryptsum(ops, fp, *argv, md)) > ret = 1; > - } else { > + else > mdprint(md, *argv, sz); > - } This is a style change and some people may thus want to have it put in a separate patch. I think combining such a minor change with this patch is fine. This patch LGTM! Cheers, Silvan > if (fp != stdin && fshut(fp, *argv)) > ret = 1; > } > -- > 2.11.0 > >
