[Rpm-maint] [PATCH] - Added support for ppc64p7 arch (Power7 and higher optimized)

[Panu Matilainen]

On 04/13/2012 01:25 PM, Phil Knirsch wrote:
> On 04/12/2012 11:48 AM, Panu Matilainen wrote:
>> On 04/11/2012 12:58 PM, Phil Knirsch wrote:
>>> Attached is the updated version of the ppc64p7 support patch. I've got
>>> rid of the AUXV env reading and doing it properly now via reading and
>>> parsing /proc/self/auxv. Additionally the patch now checks if the
>>> AT_PLATFORM is power7 or higher via a sscanf(), allowing future power
>>> architectures to benefit from it as well.
>>
>> The patch leaks file descriptors, the fd it opens is never closed.
>> In the cosmetics department you might want to split the long if with
>> sscanf() into two separate if's (or otherwise reformat) to avoid the
>> large line-wrap which is easily avoidable.
>>
>> Other than that, looks okay to me.
>>
>> - Panu -
>
> Alright, latest version attached. I've basically extracted the auxv
> parsing into a separate function, added a static struct that stores the
> info, fixed the fd closing problem and using the struct now in the
> defaultMachine().

Cool, this should be useful for various other architectures as well so 
we wont end up with a dozen slightly differing variants of 
/proc/self/auxv and /proc/cpuinfo parsing in the tree.

One more request :) Can you split the patch in two: one that adds the 
auxv parsing and another which adds the new arch bits? Oh, and I think 
it'd be useful to initialize rpmat.platform to "" so its users wont have 
to bother checking for NULL to avoid segfaulting in case the auxv 
parsing happened to fail.

	- Panu -