Factor out ip6_deletefraghdr()
ClosedPublic
Actions

Authored by kp on Feb 3 2015, 8:05 PM.

Details

Reviewers

ae
glebius

Summary

It'll be used by the pf fragment handling code.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kp updated this revision to Diff 3618.Feb 3 2015, 8:05 PM

kp retitled this revision from to Factor out ip6_deletefraghdr().

kp updated this object.

kp edited the test plan for this revision. (Show Details)

kp added a subscriber: Unknown Object (MLST).Feb 3 2015, 8:51 PM

Thanks. Patch looks good to me, but see comments. We probably need a nod from a IPv6 expert. I'll send link to ae@.

sys/netinet6/ip6_output.c
1215 ↗	(On Diff #3618)	Comment should end in dot.
1217 ↗	(On Diff #3618)	Comment should end in dot.
1218 ↗	(On Diff #3618)	Can you please use bcopy() here. And you don't need first caddr_t cast. The second cast should be changed to char *.
1223 ↗	(On Diff #3618)	Please capitalize comment and end it in dot.
1224 ↗	(On Diff #3618)	Let's add third argument 'wait' to ip6_deletefraghdr(). The argument will be passed directly to m_split().
sys/netinet6/ip6_var.h
391 ↗	(On Diff #3618)	Need single whitespace between 'mbuf' and '*'.

This revision is now accepted and ready to land.Feb 12 2015, 7:57 PM

Most of the remarks apply to the previous code as well.
I have a minor preference for the patch to be a straight move of the existing code. If you feel strongly we should to take the opportunity to do style cleanups I will of course do so.
I could also do the cleanups in a separate patch.

I have no objections against it, the code looks similar to previous implementation.

kp updated this revision to Diff 3762.Feb 14 2015, 1:36 PM

kp edited edge metadata.

This revision now requires review to proceed.Feb 14 2015, 1:36 PM

glebius accepted this revision.Feb 16 2015, 1:30 AM

glebius edited edge metadata.

This revision is now accepted and ready to land.Feb 16 2015, 1:30 AM

Damn f*ckbrikator doesn't allow me to close the revision, since I don't own it.

Kristof, looks like you will need to manually close all your revisions as I commit them. Or we can just leave some trash in this "pretty" software.

Submitted as r278828 - head/sys/netinet6

In D1764#17, @glebius wrote:

Damn f*ckbrikator doesn't allow me to close the revision, since I don't own it.

Kristof, looks like you will need to manually close all your revisions as I commit them. Or we can just leave some trash in this "pretty" software.

You need to use the full URL in your commit log in order to trigger the script that closes revisions. That's how it was setup for FreeBSD and is unrelated to the Phabricator software. I agree it shouldn't be possible to close revisions that you don't own, otherwise it would be extremely easy to screw up the system.