[Cbe-oss-dev] [PATCH] cell: abstract spu management routines
Geoff Levand
geoffrey.levand at am.sony.com
Wed Nov 8 17:03:17 EST 2006
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
More information about the cbe-oss-dev
mailing list