Re: [PATCH 02/19] powerpc: Split processor entitlement retrieval and gathering to helper routines

[Nathan Fontenot <nfont@austin.ibm.com>]

Stephen Rothwell wrote:
> Hi Robert,
> 
> On Thu, 12 Jun 2008 17:08:58 -0500 Robert Jennings <rcj@linux.vnet.ibm.com> wrote:
>> -		seq_printf(m, "R4=0x%lx\n", h_entitled);
>> -		seq_printf(m, "R5=0x%lx\n", h_unallocated);
>> -		seq_printf(m, "R6=0x%lx\n", h_aggregation);
>> -		seq_printf(m, "R7=0x%lx\n", h_resource);
> 
> This changes a user visible interface by removing the above.  I don't
> know if this matters (probably not), but it should be mentioned in the
> changelog.
>

You're right this should have been mentioned. The values it is printing 
out are the raw values returned from the H_GET_PPP hcall.  The values 
were then parsed and pretty printed afterwards.  I don't see a need to 
print these values out twice.

>> +	if (new_entitled)
>> +		*new_weight = current_weight;
>> +
>> +	if (new_weight)
>> +		*new_entitled = current_entitled;
> 
> These look fishy - checking one pointer for NULL and then updating via
> the other pointer.
> 

I thought something about this looked strange...

Unfortunately this code gets slightly updated again in patch 3/19 of 
this patch series.  The point you make is valid though, should not be 
de-referencing the pointers without validating them.

I'll update this patch with a new changelog and pull the change from 
patch 3/19 into this pach where it belongs.

-Nathan
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev