trivial mbuf api thoughts: os_mbuf_free() and os_mbuf_free_chain()

[will sanfilippo <wi...@runtime.io>]

Hello:

The os mbuf api has the following functions: os_mbuf_free() and os_mbuf_free_chain(). Both of these are currently exposed in os_mbuf.h.

I am thinking that os_mbuf_free should not be exposed; we should only expose os_mbuf_free_chain. I would always want the developer to call a function to free any mbufs chained to the mbuf being freed.

Well, actually, I would prefer os_mbuf_free() to be exposed and have that free the entire chain, but that would require chainging more code…

Yes, I know, trivial, but it is fresh in my mind :-)

Re: trivial mbuf api thoughts: os_mbuf_free() and os_mbuf_free_chain()

[Sterling Hughes <st...@apache.org>]

On 3/2/16 11:45 AM, will sanfilippo wrote:
> I was hoping that the other mbuf API exposed would be enough for users and they would never need to call os_mbuf_free. I also think that when you unlink something from a list, setting its next pointer to NULL is just good practice.
>
> I was just trying to avoid the myriad bugs I have seen in the past by having both of these api exposed. But I am fine if folks want to keep it.
>

I think we want both functions, people who work with mbufs (me!) are 
very used to m_free() and m_freem().  I was debating whether we should 
call free_chain(), freem() instead, but decided free_chain() was 
clearer.  I would be fine if people thought I was wrong, and wanted to 
rename it to freem().

Sterling

Re: trivial mbuf api thoughts: os_mbuf_free() and os_mbuf_free_chain()

[will sanfilippo <wi...@runtime.io>]

I was hoping that the other mbuf API exposed would be enough for users and they would never need to call os_mbuf_free. I also think that when you unlink something from a list, setting its next pointer to NULL is just good practice.

I was just trying to avoid the myriad bugs I have seen in the past by having both of these api exposed. But I am fine if folks want to keep it.

Will

> On Mar 2, 2016, at 11:33 AM, marko kiiskila <ma...@runtime.io> wrote:
> 
> This will be useful with any data stream where data is considered to be a
> stream of bytes, as opposed to datagrams.
> E.g. TCP sockets. TCP retransmit queue management. UNIX sockets. Pipes.
> 
>> On Mar 2, 2016, at 11:29 AM, marko kiiskila <ma...@runtime.io> wrote:
>> 
>> Freeing a single mbuf from the head of the chain is very common.
>> We should keep the API for that.
>> 
>>> On Mar 1, 2016, at 9:58 PM, paul@wrada.com wrote:
>>> 
>>> I think having a single Api mbuf_free_chain is good.  How many times have
>>> I forgot to free the chain in the past :(.
>>> 
>>> The only caveat is that the user who knowingly wants to free a single mbuf
>>> at the head of a chain (for example freeing the header that they know they
>>> added) would need to unlink it first.  To me that seems safer anyway since
>>> the programmer would need a pointer to the remainder of the chain
>>> regardless.
>>> 
>>> 
>>> 
>>> On 3/1/16, 9:32 PM, "will sanfilippo" <wi...@runtime.io> wrote:
>>> 
>>>> Hello:
>>>> 
>>>> The os mbuf api has the following functions: os_mbuf_free() and
>>>> os_mbuf_free_chain(). Both of these are currently exposed in os_mbuf.h.
>>>> 
>>>> I am thinking that os_mbuf_free should not be exposed; we should only
>>>> expose os_mbuf_free_chain. I would always want the developer to call a
>>>> function to free any mbufs chained to the mbuf being freed.
>>>> 
>>>> Well, actually, I would prefer os_mbuf_free() to be exposed and have that
>>>> free the entire chain, but that would require chainging more codeŠ
>>>> 
>>>> Yes, I know, trivial, but it is fresh in my mind :-)
>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>> 
> 

Re: trivial mbuf api thoughts: os_mbuf_free() and os_mbuf_free_chain()

[marko kiiskila <ma...@runtime.io>]

This will be useful with any data stream where data is considered to be a
stream of bytes, as opposed to datagrams.
E.g. TCP sockets. TCP retransmit queue management. UNIX sockets. Pipes.

> On Mar 2, 2016, at 11:29 AM, marko kiiskila <ma...@runtime.io> wrote:
> 
> Freeing a single mbuf from the head of the chain is very common.
> We should keep the API for that.
> 
>> On Mar 1, 2016, at 9:58 PM, paul@wrada.com wrote:
>> 
>> I think having a single Api mbuf_free_chain is good.  How many times have
>> I forgot to free the chain in the past :(.
>> 
>> The only caveat is that the user who knowingly wants to free a single mbuf
>> at the head of a chain (for example freeing the header that they know they
>> added) would need to unlink it first.  To me that seems safer anyway since
>> the programmer would need a pointer to the remainder of the chain
>> regardless.
>> 
>> 
>> 
>> On 3/1/16, 9:32 PM, "will sanfilippo" <wi...@runtime.io> wrote:
>> 
>>> Hello:
>>> 
>>> The os mbuf api has the following functions: os_mbuf_free() and
>>> os_mbuf_free_chain(). Both of these are currently exposed in os_mbuf.h.
>>> 
>>> I am thinking that os_mbuf_free should not be exposed; we should only
>>> expose os_mbuf_free_chain. I would always want the developer to call a
>>> function to free any mbufs chained to the mbuf being freed.
>>> 
>>> Well, actually, I would prefer os_mbuf_free() to be exposed and have that
>>> free the entire chain, but that would require chainging more codeŠ
>>> 
>>> Yes, I know, trivial, but it is fresh in my mind :-)
>>> 
>>> 
>>> 
>>> 
>> 
> 

Re: trivial mbuf api thoughts: os_mbuf_free() and os_mbuf_free_chain()

[marko kiiskila <ma...@runtime.io>]

Freeing a single mbuf from the head of the chain is very common.
We should keep the API for that.

> On Mar 1, 2016, at 9:58 PM, paul@wrada.com wrote:
> 
> I think having a single Api mbuf_free_chain is good.  How many times have
> I forgot to free the chain in the past :(.
> 
> The only caveat is that the user who knowingly wants to free a single mbuf
> at the head of a chain (for example freeing the header that they know they
> added) would need to unlink it first.  To me that seems safer anyway since
> the programmer would need a pointer to the remainder of the chain
> regardless.
> 
> 
> 
> On 3/1/16, 9:32 PM, "will sanfilippo" <wi...@runtime.io> wrote:
> 
>> Hello:
>> 
>> The os mbuf api has the following functions: os_mbuf_free() and
>> os_mbuf_free_chain(). Both of these are currently exposed in os_mbuf.h.
>> 
>> I am thinking that os_mbuf_free should not be exposed; we should only
>> expose os_mbuf_free_chain. I would always want the developer to call a
>> function to free any mbufs chained to the mbuf being freed.
>> 
>> Well, actually, I would prefer os_mbuf_free() to be exposed and have that
>> free the entire chain, but that would require chainging more codeŠ
>> 
>> Yes, I know, trivial, but it is fresh in my mind :-)
>> 
>> 
>> 
>> 
> 

Re: trivial mbuf api thoughts: os_mbuf_free() and os_mbuf_free_chain()

["paul@wrada.com" <pa...@wrada.com>]

I think having a single Api mbuf_free_chain is good.  How many times have
I forgot to free the chain in the past :(.

The only caveat is that the user who knowingly wants to free a single mbuf
at the head of a chain (for example freeing the header that they know they
added) would need to unlink it first.  To me that seems safer anyway since
the programmer would need a pointer to the remainder of the chain
regardless.



On 3/1/16, 9:32 PM, "will sanfilippo" <wi...@runtime.io> wrote:

>Hello:
>
>The os mbuf api has the following functions: os_mbuf_free() and
>os_mbuf_free_chain(). Both of these are currently exposed in os_mbuf.h.
>
>I am thinking that os_mbuf_free should not be exposed; we should only
>expose os_mbuf_free_chain. I would always want the developer to call a
>function to free any mbufs chained to the mbuf being freed.
>
>Well, actually, I would prefer os_mbuf_free() to be exposed and have that
>free the entire chain, but that would require chainging more codeŠ
>
>Yes, I know, trivial, but it is fresh in my mind :-)
>
>
>
>