[Bug 1001656] FreeBSD: add AF_PACKET socket familiy

[bugzilla-daemon at bugs dot ecos dot sourceware dot org]

Please do not reply to this email, use the link below.

http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656

Grant Edwards <grant.b.edwards@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |grant.b.edwards@gmail.com

--- Comment #15 from Grant Edwards <grant.b.edwards@gmail.com> ---
First, thanks for taking the time to find/fix all these bugs.

I'd like to start working on getting these committed.  After a
preliminary look through the patches I have a few suggestions on how
to more quickly get things committed.

The patches contain hundreds diffs where nothing has changed but
whitespace (usually at the end of the line):

  diff -Nur ecos-cvs-120723/packages/io/eth/current/src/net/eth_drv.c
ecos/packages/io/eth/current/src/net/eth_drv.c
  --- ecos-cvs-120723/packages/io/eth/current/src/net/eth_drv.c    2009-01-29
18:49:45.000000000 +0100
  +++ ecos/packages/io/eth/current/src/net/eth_drv.c    2012-08-02
14:30:54.000000000 +0200
  @@ -146,8 +146,8 @@
   static int
   simulate_fail( struct eth_drv_sc *sc, int which )
   {
  -    struct simulated_failure_state *s;  
  -    
  +    struct simulated_failure_state *s;
  +
       for ( s = &simulated_failure_states[0]; s <
&simulated_failure_states[2];
             s++ ) {
           if ( 0 == s->sc ) {

Having to wade through the non-changes visually searching for the real
changes makes it that much more difficult to review.  Applying the
patches and then using a diff tool like meld that can ignore
whitespace changes is one work-around, but being able to use
Bugzilla's built-in (though somewhat less intelligent) tools makes
life easier for anybody reviewing the code.

I realize that deleting trailing whitespace is a side-effect of using
certain editors and probably wasn't done intentionally.  Cleaning up
whitespace from ends of lines is fine, but it it should probably be in
a separate patch that does nothing but whitespace cleanup.

Splitting the changes up into smaller patches would also make it much
easier.  A separate patch for the AF_PACKET support and for each of
the 10 bugs you listed would be ideal.  That way somebody reviewing
the code doesn't have to try to figure out which bug a particular
change is addressing.  It also means that the work of reviewing and
committing the changes can be divided up among multiple maintainers.

Next is the question of copyright assignment.  I'm not sure where the
threshold is for "trivial" changes above which a copyright assignment
is needed, and hopefully some other maintainers to comment on
that. There's probably a good chance that for a contribution this
large a copyright assignment will be needed.

-- 
Grant

-- 
You are receiving this mail because:
You are the assignee for the bug.