[Rpm-maint] [PATCH] Cleanup: Move similar repeating code in rpmcliArgIter to rpmcliArgIterHelper.

[Panu Matilainen]

On Sat, 14 Mar 2009, Rakesh Pandit wrote:

> 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.

Ok - I didn't look at it that closely :)

>
>> - 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

Applied, thanks!

 	- Panu -