@pmatilai commented on this pull request.
> rpmRC packageBinaries(rpmSpec spec, const char *cookie, int cheating)
{
rpmRC rc = RPMRC_OK;
Package pkg;
+ Package *tasks;
+ int i = 0;
+
+ for (pkg = spec->packages; pkg != NULL; pkg = pkg->next)
+ i++;
+ tasks = xcalloc(i, sizeof(Package));
+ for (i = 0, pkg = spec->packages; pkg != NULL; i++, pkg = pkg->next)
+ tasks[i] = pkg;
+ qsort(tasks, i, sizeof(Package), compareBinaries);
Do use separate variables for indexing and things like total package count,
(re)using an 'i' for multiple different purposes only makes the code harder to
follow for now good reason. Using loop-local index variables is good way to
ensure there are no side-effects from previous rounds, and also makes it
obvious to the reader as well.
```
int npkgs = 0;
for (pkg = spec->packages; pkg != NULL; pkg = pkg->next)
npkgs++;
tasks = xcalloc(npkgs, sizeof(Package));
for (int i = 0, pkg = spec->packages; pkg != NULL; i++, pkg = pkg->next)
tasks[i] = pkg;
qsort(tasks, npkgs, sizeof(Package), compareBinaries);
```
--
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/1180#pullrequestreview-393549585_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
