Re: [RFC][PATCH v2]Add pm8001 SAS/SATA HBA driver

jack wang
2009-10-12T02:37:51Z

Grant Grundler wrote:

> Later in the patch I see:
> + pm8001_ha->tags = kmalloc(sizeof(unsigned
long)*PM8001_MAX_DEVICES,
> + GFP_KERNEL);
>
> What are we limiting to 1024 devices? LUNS?
> I noticed the constant was also used in an initialization loop too.

There are some other things that I find suspicious at this point:

-for allocations of array kcalloc() is usually the way to go as it does
overflow checks for the multiplication result (not needed here with the
current values, but doesn't really hurt in init path either)
-using sizeof(*pm8001_ha->tags) would make sure it's the correct size no
matter what type this will ever be, because ...
-unsigned long has different size on 32 and 64 bit, I don't know if that
makes
sense here. AFAICT this is sort of bitmap here so this probably doesn't need

to double it's size on 64 bit.
[Jack] Will use sizeof(*pm8001_ha->tags) instead. Thanks Eike.
Greetings,

Eike