lwip-devel
[Top][All Lists]
Advanced

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

Re: [lwip-devel] Bug in Checksum implementation and ARP fix


From: Kieran Mansley
Subject: Re: [lwip-devel] Bug in Checksum implementation and ARP fix
Date: Thu, 23 Oct 2003 09:47:46 +0100 (BST)

On Wed, 22 Oct 2003, Kenneth Porter wrote:
> > macros would be effectively hiding the cast making it harder to see what
> > was going on.  Someone coming along, reading the code, and seeing
> > "hbb2hs(var)" is not going to have a clue what it means, whereas
> > "(u16_t *)var" gives a lot more information.
> >
> > I'd vote for keeping the casts out in the open, but I feel like I must be
> > missing something as I still don't really see the problem.
>
> This is similar to suggesting that we should code in assembly language,
> because C obscures what's "really" going on behind the scenes.

Errm, no it's not.  It's suggesting that we write code that is easy for
people to understand.  I suspect most people would find assembly language
harder to understand than C.  Abstraction is not always a good thing.

> The objective is not to obscure, but to abstract. The macro (or in some
> cases, function) would be abstracting the operation of parsing a raw byte
> stream into larger objects that may require alignment for some
> architectures. The purpose is exactly that of the h2n macros used to
> convert endianness, handling architectural details and maintaining
> portability of the code, as well as allowing efficiencies on some
> architectures.
>
> Remember that the cast is an implementation detail, not an algorithm. It's
> sufficient for some architectures but not all. Extracting shorts and longs
> from byte buffers is common enough in low-level network code to use macros
> defined within architecture conditionals that use the most efficient idiom
> that's correct for that architecture.

I think I'm starting to see what you're getting at.  However, as this
seems to be an isolated case (we're not getting reports of other casts
breaking some architectures) and the long term plan is to make the
checksum routines architecture specific, it should be sufficient for the
time being to fix the existing checksum function and remind developers to
be careful when casting.

Kieran





reply via email to

[Prev in Thread] Current Thread [Next in Thread]