[dm-devel] Re: Bug in dm-stripe.c driver

[Wood, Brian J]

>-----Original Message-----
>From: Alasdair G Kergon [mailto:agk at redhat.com]
>Sent: Wednesday, November 21, 2007 11:42 AM
>To: Wood, Brian J
>Cc: device-mapper development; Ciechanowski, Ed; Healey, Douglas D
>Subject: Re: [dm-devel] Re: Bug in dm-stripe.c driver
>
>On Wed, Nov 21, 2007 at 07:39:32PM +0000, Alasdair G Kergon wrote:
>> On Wed, Nov 21, 2007 at 11:15:52AM -0800, Wood, Brian J wrote:
>> > /***** Here's the problem spot *****/
>> >        struct stripe stripe[0];
>>
>> That one must remain the last element in the struct.
>> Add any new fields above it.
>
>Here's how the real size is calculated before it's allocated:
>
>static inline struct stripe_c *alloc_context(unsigned int stripes)
>{
>        size_t len;
>
>        if (array_too_big(sizeof(struct stripe_c), sizeof(struct
stripe),
>                          stripes))
>                return NULL;
>
>        len = sizeof(struct stripe_c) + (sizeof(struct stripe) *
stripes);
>
>        return kmalloc(len, GFP_KERNEL);
>}

Ok, I did see that call to alloc_context() and it looked like the
kmalloc call was declaring enough memory to house all the drives in the
stripe (so I wasn't worried that it would overwrite anything outside the
driver's memory space). When I did my testing I noticed that if I put my
declarations above this line it worked correctly, I just thought it
might have been a logic error. 

Would it be ok to add a comment above (or below) the line "struct stripe
stripe[0];" just to prevent developers down the road from seeing this
strange behavior if they add a field?

Thanks 


>
>
>Alasdair
>--
>agk at redhat.com

Brian Wood
Software Engineer
Intel Corp., Manageability & Platform Software Division
brian.j.wood at intel.com