[openbeosnetteam] Re: IPv4 fragment reassembly

  • From: "Andrew Galante" <haiku.galante@xxxxxxxxx>
  • To: openbeosnetteam@xxxxxxxxxxxxx
  • Date: Thu, 3 Aug 2006 18:35:59 -0400

On 8/3/06, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> wrote:

Hi there,

IPv4 fragment reassembly now works as expected.

Andrew, there were quite some issues with your code, please look at the
list of changes carefully.
Beyond the ones mentioned in the commit messages already:

- you need to care about the object's lifetime a bit more, for example,
in one case, you freed items in a list, and reused the freed object to
retrieve the next item.

Oops!

- when in doubt on how to use existing API (yes, I know, it's poorly
documented beyond what the BeOS kernel offers), have a look at code
that uses the functions; it often helps a lot in how something is
supposed to be used.
For example, the hash function needs to take care of the "range"
parameter,

I did. I must have overlooked the %range on the return statement.

and it makes code harder to read when you use
list_get_first_item(), and list_get_next_item() together.
It also complicates the code a lot to test the "before" item against
NULL when you call list_insert_item_before() - because it's supposed to
just add it to the end of the list in this case (what you then did
manually via list_add_item()). This is actually clearly documented in
the list implementation, too.

- and finally, please watch your style, there is always a space between
a closing parenthesis and an opening bracket (like ") {").

I hope you're not offended when I critize your code - it wasn't bad
code; it's well understood you're new to Haiku programming, and I'm
just trying to optimize your output :-)

Thanks

If you want to know something don't hesitate to ask, even if you feel
it might be a dumb newbie question - you can only save time this way.

Bye,
   Axel.


Other related posts: