[Rpm-maint] [PATCH] Cleanup: Move similar repeating code in rpmcliArgIter to rpmcliArgIterHelper.
Rakesh Pandit
rakesh.pandit at gmail.com
Sat Mar 14 12:41:16 UTC 2009
2009/3/13 Panu Matilainen wrote:
> On Fri, 13 Mar 2009, Rakesh Pandit wrote:
[..]
Thanks for review.
> Copy-paste elimination always welcome, just some minor issues/suggestions:
>
> - Please keep alloc+free pairs "symmetric" when possible, eg
> rpmcliArgIterHelper() does rpmgiNew() so it should be freed there too,
> and as the default case in rpmcliArgIter() makes it's own rpmgiNew()
> it should also free it there. With the current patch, the
> lone rpmgiFree() in rpmcliArgIter() looks like "umm where did that
> come from?"
Very important point. Fixed.
> - Instead of having special logic for RPMDBI_PACKAGES in the helper,
> just call rpmcliArgIterHelper() with arg of NULL when it's not going
> to be used, this makes it plain obvious when the argv value is not
> used at all.
I tried this but it failed many tests.
This is because for some cases rpmgiSetArgs needs argv even if
rpmQueryVerify should have it NULL. So, for for RPMDBI_PACKAGES I had
handle it differently to keep the behaviour same.
> - The indentation of the helper is off from the general style.
>
Fixed.
Updated: http://rakesh.fedorapeople.org/rpm/0003-Cleanup-Move-similar-patterns-in-rpmcliArgIter-to-r.patch
--
Regards,
Rakesh Pandit
More information about the Rpm-maint
mailing list