Re: [Cbe-oss-dev] [PATCH] cell: abstract spu management routines

[Geoff Levand <geoffrey.levand@am.sony.com>]

Michael Ellerman wrote:
> On Tue, 2006-11-07 at 21:24 -0800, Geoff Levand wrote:
>> Michael Ellerman wrote:
>> >> +static struct platform_data *platform_data(struct spu *spu)
>> >> +{
>> >> +	BUG_ON(!spu->platform_data);
>> >> +	return (struct platform_data*)spu->platform_data;
>> >> +}
>> > 
>> > I don't see the point of this, why not just grab platform data directly?
>> 
>> Well, first, it does a check, and second, you can't just grab platform_data,
>> you need to always do the cast also. So then, is something like
>> '((struct platform_data*)spu->platform_data)->' preferred over
>> 'platform_data(spu)->'?
> 
> Yeah OK I missed the cast, I guess it's worth it then. In that case can
> you change the names? Having the struct, the member and the accessor all
> named the same is a bit confusing.
> 
> The BUG() is pretty superfluous, you're just preempting the NULL deref
> by 1 instruction.

OK, I'll do the name changes as Ben suggested.  Actually, that BUG check was
just a stale leftover from when I was getting things working :)

-Geoff