[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