Re: SV: SV: [lwip-users] Alignment problem i pbuf_alloc()

[K.J. Mansley]

On Tue, 2004-05-25 at 09:11, Lars Thorup wrote:
> Fra: K.J. Mansley [mailto:address@hidden 
> > On Tue, 2004-05-25 at 07:05, Lars Thorup wrote:
> > > This is the original code from pbuf_alloc():
> > > 
> > > case PBUF_RAM:
> > >   /* If pbuf is to be allocated in RAM, allocate memory for it. */
> > >   p = mem_malloc(MEM_ALIGN_SIZE(sizeof(struct pbuf) + length + 
> > > offset));
> > > 
> > > I have changed the third line to
> > > 
> > >   p = mem_malloc(MEM_ALIGN_SIZE(sizeof(struct pbuf) + offset) + 
> > > MEM_ALIGN_SIZE(length));
> > 
> > Would it be better to have:
> > 
> > p = mem_malloc(MEM_ALIGN_SIZE(sizeof(struct pbuf)) + 
> > MEM_ALIGN_SIZE(offset + length));
> > 
> > This makes more sense to me, as "offset" and "length" go 
> > together to make the payload of the pbuf, but it may mean the 
> > payload pointer starts off pointing at a non-word aligned 
> > address, which might cause problems?
> 
> No that would not work; I just tried it out and got the same memory
> overwrite as with the original code. The explanation is that the "offset"
> bytes is not included in the part of the buffer that the payload pointer is
> set to point at, but rather in the part before the payload pointer. This can
> be seen four lines ahead in the source code:
> 
>      p->payload = MEM_ALIGN((void *)((u8_t *)p + sizeof(struct pbuf) +
> offset));

Good point.  I think the difference boils down to this: should the start
of the possible payload (i.e. p+sizeof(struct pbuf)) be aligned, or the
initial payload pointer be aligned (i.e. p+sizeof(struct pbuf)+offset)?

If the former, the we should use the version I suggested, but also
change the last line above to:

p->payload = MEM_ALIGN((void *)((u8_t *)p + sizeof(struct pbuf))) + offset;

i.e. take the "offset" out of the align calculation.

If the later, your fix will be correct.

Could anyone with more understanding of the alignment issues than me
give an opinion on this?

Thanks

Kieran