Re: OS X compatibility patches
Julian Graham <[email protected]> Thu, 26 Mar 2015 19:12:38 -0400
| Newsgroups | gmane.network.serveez.devel |
|---|---|
| Message-ID | <CANdC_RDwPVrvO5RT4UMmacqWaYBzZbj=gOSjw_htazfz08WAMw@mail.gmail.com> |
--===============8066616377495368623==
Content-Type: multipart/alternative; boundary=001a11362d98e590fe051239264f
--001a11362d98e590fe051239264f
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
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:
*SNIP*
@@ -451,9 +451,11 @@
static void
collect (void)
{
int numreqs =3D 16;
+ size_t iflen =3D sizeof (struct ifreq);
+
struct ifconf ifc;
- struct ifreq *ifr;
struct ifreq ifr2;
int n;
int fd;
@@ -502,9 +504,10 @@ collect (void)
break;
}
- ifr =3D ifc.ifc_req;
- for (n =3D 0; n < ifc.ifc_len; n +=3D sizeof (struct ifreq), ifr++)
+ for (n =3D 0; n < ifc.ifc_len; n +=3D iflen)
{
+ struct ifreq *ifr =3D ifc.ifc_req + n;
+
*SNIP*
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?
Thanks,
Julian
On Tue, Feb 24, 2015 at 3:46 AM, Thien-Thi Nguyen <[email protected]> wrote:
> () Julian Graham <[email protected]>
> () Mon, 23 Feb 2015 23:05:28 -0500
>
> > [=E2=80=98for=E2=80=99 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 =E2=80=98for=E2=80=99 "header" position) and i=
nstead
> 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)))
> =3D> nil
>
--001a11362d98e590fe051239264f
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr">Hey ttn,<div><br></div><div>It's been a while since I&=
#39;ve had access to a Mac OS X machine and cycles to spend on compatibilit=
y work, so please forgive my delayed reponse!</div><div><br></div><div>I un=
derstand 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. Wh=
ere 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=
:</div><div><br></div><div>*SNIP*</div><div><br></div>@@ -451,9 +451,11 @@=
=C2=A0<div>=C2=A0static void<br>=C2=A0collect (void)<br>=C2=A0{<br>=C2=A0 =
=C2=A0int numreqs =3D 16;<br>+ =C2=A0size_t iflen =3D sizeof (struct ifreq)=
;<br>+<br>=C2=A0 =C2=A0struct ifconf ifc;<br>- =C2=A0struct ifreq *ifr;<br>=
=C2=A0 =C2=A0struct ifreq ifr2;<br><br>=C2=A0 =C2=A0int n;<br>=C2=A0 =C2=A0=
int fd;<br><br>@@ -502,9 +504,10 @@ collect (void)<br>=C2=A0 =C2=A0 =C2=A0 =
=C2=A0break;<br>=C2=A0 =C2=A0 =C2=A0}<br>=C2=A0<br>- =C2=A0ifr =3D ifc.ifc_=
req;<br>- =C2=A0for (n =3D 0; n < ifc.ifc_len; n +=3D sizeof (struct ifr=
eq), ifr++)<br>+ =C2=A0for (n =3D 0; n < ifc.ifc_len; n +=3D iflen)<br>=
=C2=A0 =C2=A0 =C2=A0{<br>+ =C2=A0 =C2=A0 =C2=A0struct ifreq *ifr =3D ifc.if=
c_req + n;<br>+ =C2=A0 =C2=A0 =C2=A0<div><br></div></div><div>*SNIP*</div><=
div><br></div><div>Does that get at what you were talking about? If so, gre=
at! I'll package it up into a real patch. If not, can you lead me throu=
gh it a bit more explicitly?</div><div><br></div><div><br></div><div>Thanks=
,</div><div>Julian</div><div><br></div></div><div class=3D"gmail_extra"><br=
><div class=3D"gmail_quote">On Tue, Feb 24, 2015 at 3:46 AM, Thien-Thi Nguy=
en <span dir=3D"ltr"><<a href=3D"mailto:[email protected]" target=3D"_blank">t=
[email protected]</a>></span> wrote:<br><blockquote class=3D"gmail_quote" style=
=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">() Julia=
n Graham <<a href=3D"mailto:[email protected]">[email protected]</a>>=
<br>
() Mon, 23 Feb 2015 23:05:28 -0500<br>
<br>
=C2=A0 =C2=A0> [=E2=80=98for=E2=80=99 transform]<br>
<span class=3D""><br>
=C2=A0 =C2=A0It took me a moment to remember, but:<br>
<br>
=C2=A0 =C2=A0In the original code, `ifc.ifc_len' is always a multiple o=
f<br>
=C2=A0 =C2=A0`sizeof (struct ifreq)', and so `n' is incremented in =
the<br>
=C2=A0 =C2=A0UPDATE as many times as there are interfaces in the<br>
=C2=A0 =C2=A0buffer. `ifr' is just a pointer version of `n'.<br>
<br>
=C2=A0 =C2=A0The new code acknowledges that the interface structures in<br>
=C2=A0 =C2=A0`ifc' may have different lengths, and so `n' and `ifr&=
#39; need<br>
=C2=A0 =C2=A0to be incremented by the length of each structure in the<br>
=C2=A0 =C2=A0buffer, an operation complex enough that I moved it out of<br>
=C2=A0 =C2=A0the UPDATE part of the `for' loop. Think of it as handling=
a<br>
=C2=A0 =C2=A0more general case than the original code.<br>
<br>
</span>Right.=C2=A0 I understand the generalization thrust.=C2=A0 The doubt=
<br>
revolves around the precise timing of UPDATE wrt NON-OSX-PATH.<br>
IIUC, before-patch, we have:<br>
<br>
=C2=A0for (INIT; GATE; UPDATE)<br>
=C2=A0 =C2=A0{<br>
=C2=A0 =C2=A0 =C2=A0NON-OSX-PATH;<br>
=C2=A0 =C2=A0}<br>
<br>
which means that UPDATE is performed *after* NON-OSX-PATH, and<br>
after-patch, we have:<br>
<br>
=C2=A0for (INIT; GATE; )<br>
=C2=A0 =C2=A0{<br>
=C2=A0#if OSX<br>
=C2=A0 =C2=A0 =C2=A0OSX-PATH;<br>
=C2=A0#else<br>
=C2=A0 =C2=A0 =C2=A0UPDATE;<br>
=C2=A0#endif<br>
=C2=A0 =C2=A0 =C2=A0NON-OSX-PATH;<br>
=C2=A0 =C2=A0}<br>
<br>
which means that UPDATE is performed *before* NON-OSX-PATH.<br>
Maybe i'm (still) missing something (coffee underflow error)?<br>
<br>
I think i would be more inclined to accept a change that keeps<br>
UPDATE where it is (in the =E2=80=98for=E2=80=99 "header" positio=
n) and instead<br>
introduces a local variable for the entry length, w/ a default<br>
constant value.=C2=A0 This would be the "preparation" patch.<br>
<br>
The follow-on "payload" patch would then add OSX-PATH proper,<br>
including dynamic update of that variable.=C2=A0 (Bonus points for<br>
not requiring the #else branch.)<br>
<br>
In this way, all steps can be more easily recognized as correct<br>
by programmers of disparate abilities, experience and mindset.<br>
<div class=3D"HOEnZb"><div class=3D"h5"><br>
--<br>
Thien-Thi Nguyen<br>
=C2=A0 =C2=A0GPG key: 4C807502<br>
=C2=A0 =C2=A0(if you're human and you know it)<br>
=C2=A0 =C2=A0 =C2=A0 read my lisp: (responsep (questions 'technical)<br=
>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(not (via 'mailing-list)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0=3D> nil<br>
</div></div></blockquote></div><br></div>
--001a11362d98e590fe051239264f--
--===============8066616377495368623==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
dev-serveez mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/dev-serveez
--===============8066616377495368623==--