Hi, Zhao.
zhao, forrest wrote:
> On Wed, 2006-05-10 at 10:44 +0900, Tejun Heo wrote:
>> Hello, all.
> Tejun,
>
> I have comments about definition of macro ata_link_for_each_dev(dev,
> link) and struct ata_port{};.
>
> In the definition of struct ata_port{}, there's
> ......
> struct ata_link link;
> struct ata_device __dev1;
> ......
>
> Then macro ata_link_for_each_dev() assumes that the field 'device' in
> struct ata_link is adjacent to field '__dev1' in struct ata_port.
>
> I think this assumption is not correct in theory. Because the alignment
> may make these two fields not adjacent in memory.
>
> Although we haven't found the problem so far, it's very dangerous to
> have such assumption in the code.
Well, I think the technique is pretty widespread w/ flexible array
member, which is C99 standard and before C99 GCC had zero length array
for the same purpose. e.g.
struct asdf {
int nr_entries;
void *ar[];
};
struct asdf *p;
p = kmalloc(sizeof(*p) + nr * sizeof(p->ar[0], GFP_KERNEL);
if (p)
p->nr_entries = nr;
This works because alignof(outer struct) >= alignof(nested struct). So,
we might end up allocating a few extra bytes due to alignment
requirements but that's perfectly okay.
__dev1 is the same. It might or might not be allocated right after
link->dev[0] but it's guaranteed that &link->dev[1] <= &link->__dev1.
So, no problem there. I'm pretty sure that this kind of technique is
used in other parts of the kernel too.
--
tejun
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
