[Top][All Lists]

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

Re: [dev-serveez] OS X compatibility patches

From: Julian Graham
Subject: Re: [dev-serveez] OS X compatibility patches
Date: Thu, 26 Mar 2015 19:12:38 -0400

Hey ttn,

It's been a while since I've had access to a Mac OS X machine and cycles to spend on compatibility work, so please forgive my delayed reponse!

I understand the desire to split up patch 0005 into something more digestible, but I'm not sure I fully understand your suggestion on how to do it. Where should that local variable be introduced, and which counter within the `for' clause should it replace? In case it's faster for you to look at source code, here's my attempt at the "preparation" patch:


@@ -451,9 +451,11 @@ 
 static void
 collect (void)
   int numreqs = 16;
+  size_t iflen = sizeof (struct ifreq);
   struct ifconf ifc;
-  struct ifreq *ifr;
   struct ifreq ifr2;

   int n;
   int fd;

@@ -502,9 +504,10 @@ collect (void)
-  ifr = ifc.ifc_req;
-  for (n = 0; n < ifc.ifc_len; n += sizeof (struct ifreq), ifr++)
+  for (n = 0; n < ifc.ifc_len; n += iflen)
+      struct ifreq *ifr = ifc.ifc_req + n;


Does that get at what you were talking about? If so, great! I'll package it up into a real patch. If not, can you lead me through it a bit more explicitly?


On Tue, Feb 24, 2015 at 3:46 AM, Thien-Thi Nguyen <address@hidden> wrote:
() 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:


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

 for (INIT; GATE; )
 #if OSX

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

reply via email to

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