Re: question about mbuf intialization

[Beverly Schwartz <bschwart%bbn.com@localhost>]

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.

For anyone developing new code, they may do it the "old" way where they 
maintain the mbuf lists and figure out if they need extra storage and set 
lengths accordingly, or they may rely on m_copyback.  So while this isn't a big 
issue for code that exists, it's a headache for future development.

I didn't painstakingly go through every architecture and device driver, but I 
would be willing to be that at least one is buggy because of lack of 
initialization of m_len and/or m_pkthdr.len.

Finally, using rump as a way to test kernel code in user space is really 
powerful.  I did a lot of exercising of mbuf code, and initially, I did a lot 
of crashing and burning because I didn't realize that m_len wasn't initialized. 
 This became a pain to deal with - everytime I got an mbuf I had to remember to 
initialize m_len and m_pkthdr.len.  So I just went and changed the source code, 
and this discussion has resulted.

-Bev