@ignatenkobrain commented on this pull request.
LGTM with small nitpicks
> goto err;
}
+
+ if (threads > 0)
If I read https://facebook.github.io/zstd/zstd_manual.html correctly, even if
you set it to `0`, it will use single thread which makes this if unneeded.
> @@ -1100,7 +1101,12 @@ static rpmzstd rpmzstdNew(int fdno, const char *fmode)
flags &= ~O_ACCMODE;
flags |= O_RDWR;
continue;
- break;
+ case 'T':
I think it would be nice to support setting it to -1, same as for lzma:
```
if (threads == -1)
threads = rpmExpandNumeric("%{getncpus}");
```
> + if (c >= (int)'0' && c <= (int)'9')
+ threads = strtol(s-1, (char **)&s, 10);
Just curious if it would make sense to change lzopen_internal() to follow the
same logic? If so, could you push a separate commit for that?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1303#pullrequestreview-446799809_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
