Re: [dev-serveez] OS X compatibility patches

[Thien-Thi Nguyen]

() Julian Graham <address@hidden>
() Mon, 23 Feb 2015 23:05:28 -0500

   > [‘for’ transform]

   It took me a moment to remember, but:

   In the original code, `ifc.ifc_len' is always a multiple of
   `sizeof (struct ifreq)', and so `n' is incremented in the
   UPDATE as many times as there are interfaces in the
   buffer. `ifr' is just a pointer version of `n'.

   The new code acknowledges that the interface structures in
   `ifc' may have different lengths, and so `n' and `ifr' need
   to be incremented by the length of each structure in the
   buffer, an operation complex enough that I moved it out of
   the UPDATE part of the `for' loop. Think of it as handling a
   more general case than the original code.

Right.  I understand the generalization thrust.  The doubt
revolves around the precise timing of UPDATE wrt NON-OSX-PATH.
IIUC, before-patch, we have:

 for (INIT; GATE; UPDATE)
   {
     NON-OSX-PATH;
   }

which means that UPDATE is performed *after* NON-OSX-PATH, and
after-patch, we have:

 for (INIT; GATE; )
   {
 #if OSX
     OSX-PATH;
 #else
     UPDATE;
 #endif
     NON-OSX-PATH;
   }

which means that UPDATE is performed *before* NON-OSX-PATH.
Maybe i'm (still) missing something (coffee underflow error)?

I think i would be more inclined to accept a change that keeps
UPDATE where it is (in the ‘for’ "header" position) and instead
introduces a local variable for the entry length, w/ a default
constant value.  This would be the "preparation" patch.

The follow-on "payload" patch would then add OSX-PATH proper,
including dynamic update of that variable.  (Bonus points for
not requiring the #else branch.)

In this way, all steps can be more easily recognized as correct
by programmers of disparate abilities, experience and mindset.

-- 
Thien-Thi Nguyen
   GPG key: 4C807502
   (if you're human and you know it)
      read my lisp: (responsep (questions 'technical)
                               (not (via 'mailing-list)))
                     => nil

signature.asc
Description: PGP signature