tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: question about mbuf intialization



On Sep 23, 2013, at 3:15 PM, Iain Hibbert wrote:

> On Fri, 20 Sep 2013, Beverly Schwartz wrote:
> 
>> 
>> On Sep 20, 2013, at 3:48 PM, Iain Hibbert <plunky%ogmig.net@localhost> wrote:
>> 
>>> On Fri, 20 Sep 2013, Beverly Schwartz wrote:
>>> 
>>>> 
>>>> On Sep 19, 2013, at 4:21 PM, Christos Zoulas 
>>>> <christos%astron.com@localhost> wrote:
>>>> 
>>>>> In article <546B8CEC-0675-463F-B5C8-6A0FD5541B83%bbn.com@localhost>,
>>>>> Beverly Schwartz  <bschwart%bbn.com@localhost> wrote:
>>>>>> 
>>>>>> Any reason why we can't add
>>>>>>  m->m_len = 0;
>>>>>> to m_get, and
>>>>>>  m->m_pkthdr.len = 0;
>>>>>> to m_gethdr?
>>>>> 
>>>>> Makes sense, but at the same time we should remove the superfluous zeroing
>>>>> from the other places...
>>>> 
>>>> A quick scan of the code shows that some of the time m_len is set to 0,
>>>> but more of the time, the library using it just sets it to its ultimate
>>>> value.
>>> 
>>> why does it seem useful to always set it to zero, if more often than not
>>> it will be immediately set to another value..?
>> 
>> There are various goals with software development:
>> - efficiency,
>> - robustness,
>> - maintainability
>> 
>> I'm concerned primarily about maintainability, with a second concern with 
>> robustness.
>> 
>> Not initializing one particular field in a structure when all others are
>> initialized is error-prone, especially since the mbuf man page doesn't
>> say anything about the user needing to initialize them.  In addition,
>> there are functions in mbuf that are completely reasonable to use such
>> as m_copyback and m_adj that depend on m_len being set.
> 
> The manpage says that "internal data" is initialized by the allocation
> routines, but it doesn't say what that is. That could be improved no
> doubt. The manpage is pretty minimal anyway, which makes a steep learning
> curve, and I surely recall that I had to look at the header files and/or
> source a lot when I wrote the Bluetooth code, but I personally do not feel
> that it is at all onerous to set the length of the data area when that is
> loaded.
> 
> I would expect that if code exists that does not initialize the length
> value, then it has not been tested and will not ever work. The mbufs are
> likely to have been used before and *will* contain random values. (I don't
> know how rump deals with allocations like this, if the data would be
> zeroed by default?)


Rump uses the actual kernel code, therefore, just like the kernel,
m_len will contain what every value was last there, or some random
scribble from memory.  Writing a rump unit test, there's a lot of
grabbing mbufs and trying out different things.  I can set m_len and
m_pkthdr.len, but it's a nuisance to remember to have to do it with
each allocation of an mbuf.

-Bev


Home | Main Index | Thread Index | Old Index