Re: [Qemu-devel] [PATCH for-3.0] slirp: Correct size check in m_inc()

[Peter Maydell]

On 9 August 2018 at 22:54, Samuel Thibault <address@hidden> wrote:
> Dr. David Alan Gilbert, le jeu. 09 août 2018 12:32:05 +0100, a ecrit:
>> >    |--datasize---->|---m_len------->
>> >    |----------m_size------------------------------>
>> >                    |----M_ROOM-------------------->
>> >                                     |-M_FREEROOM-->
>> >
>> >    ^               ^                               ^
>> >    m_dat           m_data                          end of buffer
>> >
>> > ("datasize" is a bit misnamed, as it's "size of the leading
>> > gap between the start of the buffer and the data"; "gapsize"
>> > would be more helpful.)
>> >
>> > Anyway, we allocate size + datasize, and
>> > m_size == datasize + M_ROOM. We know that size >= M_ROOM,
>> > so the allocated buffer must be at least m_size big.
>>
>> Ah OK, thanks.
>> (That ascii art could do with being in a comment somewhere!)
>
> Indeed. Peter, maybe your Signed-off-by on this? :)

Signed-off-by: Peter Maydell <address@hidden>

> Samuel
>
> commit 4be85a1eeb6b19e91491e689d4d0d054030cbb49
> Author: Peter Maydell <address@hidden>
> Date:   Thu Aug 9 23:52:59 2018 +0200
>
>     slirp: document mbuf pointers and sizes
>
>     Signed-off-by: Samuel Thibault <address@hidden>
>
> diff --git a/slirp/mbuf.h b/slirp/mbuf.h
> index 33b84485d6..a5bb3f9e66 100644
> --- a/slirp/mbuf.h
> +++ b/slirp/mbuf.h
> @@ -47,6 +47,16 @@
>   * free the m_ext.  This is inefficient memory-wise, but who cares.
>   */
>
> +/*
> + *  |--gapsize----->|---m_len------->
> + *  |----------m_size------------------------------>
> + *                  |----M_ROOM-------------------->
> + *                                   |-M_FREEROOM-->
> + *
> + *  ^               ^                               ^
> + *  m_dat/m_ext     m_data                          end of buffer
> + */
> +

...but
(a) you should add a comment describing what 'gapsize'
is, ie that there may be a gap between the in-use data and the
start of the allocated buffer, and
(b) m_inc() should change its variable name to match.

thanks
-- PMM