On Sat, Apr 07, 2018 at 08:15:55PM +0530, Aditya Xavier wrote:
> That explains everything. However, one question. 
> 
> When we do a os_mbuf_free_chain, shouldn’t om_data also provide “”, instead 
> of the previous value ?

The contents of an mbuf's data buffer beyond `om_len` are always
indeterminate.  The issue here is that the app prints the mbuf data
despite `om_len` being equal to 0.

What is actually happening is `os_mbuf_get()` is returning the same mbuf
that was just freed.  This is a consequence of Mynewt's mempool
implementation; elements are allocated in the reverse order they were
freed.  Since the app only allocates one mbuf at a time, the mempool
always returns a pointer to the same mbuf.

The mbufpool implementation could zero out an mbuf when it gets
allocated.  This would prevent the app from printing stale contents.
However, this behavior would be a waste of time; setting `om_len` to 0
is sufficient as long as the app respects the mbuf API.

Chris

> 
> I understand its not wise to delete the data from the memory for the sake of 
> efficiency, however was wondering if thats the expected result.
> 
> Thanks for the explanation though. Would change the code accordingly.
> 
> Thanks,
> Aditya Xavier.
> 
> 
> > On 07-Apr-2018, at 8:55 AM, Christopher Collins <ch...@runtime.io> wrote:
> > 
> > Hi Aditya,
> > 
> > On Sat, Apr 07, 2018 at 07:59:44AM +0530, Aditya Xavier wrote:
> >> Hi Christopher,
> >> 
> >> That is the expected behaviour, however if you try running the sample app 
> >> I gave you would notice the following 
> >> After step 11, I.e initialise the os_mbuf by doing a os_mbuf_get from a 
> >> mempool, the new value is overwritten on the previous value.
> >> 
> >> I.e
> >> 1. Accessing the mbuf after doing a free chain, om_data still holds the 
> >> value.
> >> 2. Initialising it again by doing a os_mbuf_get is not getting me a clean 
> >> mbuf, rather it holds the previous value and om_mbuf_copyinto merely 
> >> overwrites it. So incase the new string length is smaller, om_data would 
> >> return wrong value.
> >> 
> >> Am sorry if am not able to explain it properly however I would appreciate 
> >> it if you can test the app once.
> > 
> > I ran your app, and I see nothing unusual in the output.  Here is what I
> > get:
> > 
> >    (gdb) r
> >    Starting program:
> >    
> > /mnt/data2/work/micosa/repos/mynewt-core/bin/targets/blinky-sim/app/apps/blinky/blinky.elf
> >    uart0 at /dev/pts/16
> >    UART MBUF Created 1 to 1
> >    Received Value :- abc
> >    Received Length :- 3
> >    Value after Reinit :- abc
> >    Length after Reinit :- 0
> >    Received Value :- hello
> >    Received Length :- 5
> >    Value after Reinit :- hello
> >    Length after Reinit :- 0
> >    Received Value :- gagao
> >    Received Length :- 4
> >    Value after Reinit :- gagao
> >    Length after Reinit :- 0
> > 
> > To get this output, I typed the following strings into the console:
> > 
> >    abc
> >    hello
> >    gaga
> > 
> > If I understand correctly, your concern is the following part of the
> > output:
> > 
> >    Received Value :- gagao
> >    Received Length :- 4
> >    Value after Reinit :- gagao
> >    Length after Reinit :- 0
> > 
> > Specifically, you are unsure why:
> > 
> >    * The first line contains "gagao" rather than "gaga".
> >    * The third line contains "gagao" rather than "".
> > 
> > Your program uses the `%s` format specifier to print the contents of an
> > mbuf.  This is probably not what you want, for a number of reasons:
> > 
> >    * Mbuf contents are not typically null-terminated.
> >    * Mbuf contents are not guaranteed to be contiguous (i.e., multiple
> >      bufers may be chained).
> > 
> > Here is a reliable, if inefficient, way to print an mbuf as a string:
> > 
> >    static void
> >    print_mbuf(const struct os_mbuf *om)
> >    {
> >        int i;
> > 
> >        for (; om != NULL; om = SLIST_NEXT(om, om_next)) {
> >            for (i = 0; i < om->om_len; i++) {
> >                putchar(om->om_data[i]);
> >            }
> >        }
> >    }
> > 
> > If you are sure the mbuf is not chained, then you don't need the outer
> > loop.
> > 
> > Chris
> 

Reply via email to