Date: Sat, 21 Sep 2002 01:00:02 +1000 From: Darren Reed <darrenr@reed.wattle.id.au> To: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern uipc_mbuf.c Message-ID: <200209201500.BAA25430@avalon.reed.wattle.id.au>
next in thread | raw e-mail | index | archive | help
>From Poul-Henning Kamp : > In some mail, Julian Elischer writes: > > > > Modified files: > > sys/kern uipc_mbuf.c > > Log: > > While well intentionned the check to see it there is a packet > > header and return that length, was misguided. > > > > The check itself didn't take into account the fact that the > > mbuf pointer pased in may be null, and the function is > > defined specifically for cases where the caller knows what it wants. > > Rather than fix the check I'm removing it as phk suggested. > > Not to mention that it failed to perform the other half of m_lenght()'s > job: return a pointer to the last mbuf in the chain. Well, what is the purpose of m_length() ? Is there a comment supplied with it that says what it is meant to do ? Or meant to provide when it exits ? I don't know who wrote it, or some of the other functions in uipc_mbuf.c, but the level of documentation about what they're meant to do is rather poor and should be improved upon by those in the know so that people who want to make changes are properly armed with the appropriate knowledge. Saying "RTFS" as an answer to the above is not acceptable. I don't care what anyone says, code is NOT self documenting. Why isn't there at least a copy of the CVS comment for the commit above the function m_length() ? Is the author of it _that_ lazy ? btw, I think m_length() as a function which does something besides return the length of an mbuf is wrong. I think that kind of task is better delt with by keeping a pointer to the last mbuf in the chain. I'd recommend that m_length() do what it suggests it do and have another one, m_lastbuf() or something to do that job. Well, no, needing m_lastbuf() just sucks bigtime. Some thought to making m_length() a M_LENGTH() might be an idea too (that thought crossed my mind elsewhere), but that'd only fit well if mbufs were fixed up with a tail pointer. Someone else mentioned that m_pkthdr.len being the true length of the mbuf chain when M_PKTHDR is set is not gauranteed to be correct in the case of some sockbufs. That sounds like a "bug waiting to happen", if true. Darren p.s. Poul, this email was crafted just for you because I know you love to get in there and insult people, left right & center, in all manner of flames. Bet you can't resist this one :-) As for me, I'm much stronger willed than you. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200209201500.BAA25430>