Mailing List Archive

Re: Another wackamole embedded perl patch
Glenn Nielsen wrote:

>Hi Theo,
>
>Attached is a patch to fix the instantiation of the perl hash variables
>which are passed to the dynamic perl functions. The hash key length
>used as the third arg to hv_store needs to be the actual string length
>without the terminating '\0', not the size required for storing in
>memory with the terminating '\0'. Without the patch the hash keys
>"ifname", "ip", "broadcast", and "netmask" were not keys in the
>hash passed to the perl function.
>
>Regards,
>
>Glenn
>
>----------------------------------------------------------------------
>Glenn Nielsen glenn@more.net | /* Spelin donut madder |
>MOREnet System Programming | * if iz ina coment. |
>Missouri Research and Education Network | */ |
>----------------------------------------------------------------------
>
>
>------------------------------------------------------------------------
>
>--- perl.c.orig Thu Apr 22 14:44:47 2004
>+++ perl.c Thu Nov 17 09:58:28 2005
>@@ -102,26 +102,26 @@
> SAVETMPS; /* ...is a temporary variable. */
> PUSHMARK(SP); /* remember the stack pointer */
> phv = newHV();
>- hv_store(phv, "ifname", sizeof("ifname"), sv_2mortal(newSVpv(p->ifname,0)),0);
>- hv_store(phv, "ip", sizeof("ip"), sv_2mortal(newSVpv(inet_ntoa(p->ipaddr),0)),0);
>- hv_store(phv, "broadcast", sizeof("broadcast"), sv_2mortal(newSVpv(inet_ntoa(p->bcast),0)),0);
>- hv_store(phv, "netmask", sizeof("netmask"), sv_2mortal(newSVpv(inet_ntoa(p->netmask),0)),0);
>+ hv_store(phv, "ifname", strlen("ifname"), sv_2mortal(newSVpv(p->ifname,0)),0);
>+ hv_store(phv, "ip", strlen("ip"), sv_2mortal(newSVpv(inet_ntoa(p->ipaddr),0)),0);
>+ hv_store(phv, "broadcast", strlen("broadcast"), sv_2mortal(newSVpv(inet_ntoa(p->bcast),0)),0);
>+ hv_store(phv, "netmask", strlen("netmask"), sv_2mortal(newSVpv(inet_ntoa(p->netmask),0)),0);
>
> eav = newAV();
> for(i = 0; i < MAX_DEP_IF && e[i].ipaddr.s_addr != 0; i++) {
> HV* tmphv;
> tmphv = newHV();
>- hv_store(tmphv, "ifname", sizeof("ifname"), sv_2mortal(newSVpv(e[i].ifname,0)),0);
>- hv_store(tmphv, "ip", sizeof("ip"), sv_2mortal(newSVpv(inet_ntoa(e[i].ipaddr),0)),0);
>- hv_store(tmphv, "broadcast", sizeof("broadcast"), sv_2mortal(newSVpv(inet_ntoa(e[i].bcast),0)),0);
>- hv_store(tmphv, "netmask", sizeof("netmask"), sv_2mortal(newSVpv(inet_ntoa(e[i].netmask),0)),0);
>+ hv_store(tmphv, "ifname", strlen("ifname"), sv_2mortal(newSVpv(e[i].ifname,0)),0);
>+ hv_store(tmphv, "ip", strlen("ip"), sv_2mortal(newSVpv(inet_ntoa(e[i].ipaddr),0)),0);
>+ hv_store(tmphv, "broadcast", strlen("broadcast"), sv_2mortal(newSVpv(inet_ntoa(e[i].bcast),0)),0);
>+ hv_store(tmphv, "netmask", strlen("netmask"), sv_2mortal(newSVpv(inet_ntoa(e[i].netmask),0)),0);
> av_push(eav, (SV*)sv_2mortal(newRV_inc((SV *)tmphv)));
> }
> rhv = newHV();
>- hv_store(rhv, "ifname", sizeof("ifname"), sv_2mortal(newSVpv(r->ifname,0)),0);
>- hv_store(rhv, "ip", sizeof("ip"), sv_2mortal(newSVpv(inet_ntoa(r->ipaddr),0)),0);
>- hv_store(rhv, "broadcast", sizeof("broadcast"), sv_2mortal(newSVpv(inet_ntoa(r->bcast),0)),0);
>- hv_store(rhv, "netmask", sizeof("netmask"), sv_2mortal(newSVpv(inet_ntoa(r->netmask),0)),0);
>+ hv_store(rhv, "ifname", strlen("ifname"), sv_2mortal(newSVpv(r->ifname,0)),0);
>+ hv_store(rhv, "ip", strlen("ip"), sv_2mortal(newSVpv(inet_ntoa(r->ipaddr),0)),0);
>+ hv_store(rhv, "broadcast", strlen("broadcast"), sv_2mortal(newSVpv(inet_ntoa(r->bcast),0)),0);
>+ hv_store(rhv, "netmask", strlen("netmask"), sv_2mortal(newSVpv(inet_ntoa(r->netmask),0)),0);
>
> XPUSHs((SV*)sv_2mortal(newRV_inc((SV *)phv)));/* push the psuedo interface onto the stack */
> XPUSHs((SV*)sv_2mortal(newRV_inc((SV *)eav)));/* push the extra interface array ref onto the stack */
>
>


Made a slightly different change. No since in calling strlen on
something when your compiler can make it a static numner -- as long as I
don't give it the wrong static number ;-)

; cvs diff -u
cvs diff: Diffing .
Index: ChangeLog
===================================================================
RCS file: /storage/cvs/munjal/wackamole/ChangeLog,v
retrieving revision 1.27
diff -u -r1.27 ChangeLog
--- ChangeLog 18 Nov 2005 16:49:05 -0000 1.27
+++ ChangeLog 18 Nov 2005 16:53:22 -0000
@@ -89,3 +89,5 @@
* patch from < pete at more dot net > and < anderson at more dot net >
regarding uninitialized stack variable usable. Fixes occasional
interface operations on FreeBSD.
+ * wrong string length of static strings, off-by-one. from Glenn Nielsen
+ < glenn at maildot more dot net >
Index: perl.c
===================================================================
RCS file: /storage/cvs/munjal/wackamole/perl.c,v
retrieving revision 1.4
diff -u -r1.4 perl.c
--- perl.c 7 May 2005 20:23:27 -0000 1.4
+++ perl.c 18 Nov 2005 16:53:22 -0000
@@ -84,26 +84,26 @@
SAVETMPS; /* ...is a temporary
variable. */
PUSHMARK(SP); /* remember the stack
pointer */
phv = newHV();
- hv_store(phv, "ifname", sizeof("ifname"),
sv_2mortal(newSVpv(p->ifname,0)),0);
- hv_store(phv, "ip", sizeof("ip"),
sv_2mortal(newSVpv(inet_ntoa(p->ipaddr),0)),0);
- hv_store(phv, "broadcast", sizeof("broadcast"),
sv_2mortal(newSVpv(inet_ntoa(p->bcast),0)),0);
- hv_store(phv, "netmask", sizeof("netmask"),
sv_2mortal(newSVpv(inet_ntoa(p->netmask),0)),0);
+ hv_store(phv, "ifname", sizeof("ifname")-1,
sv_2mortal(newSVpv(p->ifname,0)),0);
+ hv_store(phv, "ip", sizeof("ip")-1,
sv_2mortal(newSVpv(inet_ntoa(p->ipaddr),0)),0);
+ hv_store(phv, "broadcast", sizeof("broadcast")-1,
sv_2mortal(newSVpv(inet_ntoa(p->bcast),0)),0);
+ hv_store(phv, "netmask", sizeof("netmask")-1,
sv_2mortal(newSVpv(inet_ntoa(p->netmask),0)),0);

eav = newAV();
for(i = 0; i < MAX_DEP_IF && e[i].ipaddr.s_addr != 0; i++) {
HV* tmphv;
tmphv = newHV();
- hv_store(tmphv, "ifname", sizeof("ifname"),
sv_2mortal(newSVpv(e[i].ifname,0)),0);
- hv_store(tmphv, "ip", sizeof("ip"),
sv_2mortal(newSVpv(inet_ntoa(e[i].ipaddr),0)),0);
- hv_store(tmphv, "broadcast", sizeof("broadcast"),
sv_2mortal(newSVpv(inet_ntoa(e[i].bcast),0)),0);
- hv_store(tmphv, "netmask", sizeof("netmask"),
sv_2mortal(newSVpv(inet_ntoa(e[i].netmask),0)),0);
+ hv_store(tmphv, "ifname", sizeof("ifname")-1,
sv_2mortal(newSVpv(e[i].ifname,0)),0);
+ hv_store(tmphv, "ip", sizeof("ip")-1,
sv_2mortal(newSVpv(inet_ntoa(e[i].ipaddr),0)),0);
+ hv_store(tmphv, "broadcast", sizeof("broadcast")-1,
sv_2mortal(newSVpv(inet_ntoa(e[i].bcast),0)),0);
+ hv_store(tmphv, "netmask", sizeof("netmask")-1,
sv_2mortal(newSVpv(inet_ntoa(e[i].netmask),0)),0);
av_push(eav, (SV*)sv_2mortal(newRV_inc((SV *)tmphv)));
}
rhv = newHV();
- hv_store(rhv, "ifname", sizeof("ifname"),
sv_2mortal(newSVpv(r->ifname,0)),0);
- hv_store(rhv, "ip", sizeof("ip"),
sv_2mortal(newSVpv(inet_ntoa(r->ipaddr),0)),0);
- hv_store(rhv, "broadcast", sizeof("broadcast"),
sv_2mortal(newSVpv(inet_ntoa(r->bcast),0)),0);
- hv_store(rhv, "netmask", sizeof("netmask"),
sv_2mortal(newSVpv(inet_ntoa(r->netmask),0)),0);
+ hv_store(rhv, "ifname", sizeof("ifname")-1,
sv_2mortal(newSVpv(r->ifname,0)),0);
+ hv_store(rhv, "ip", sizeof("ip")-1,
sv_2mortal(newSVpv(inet_ntoa(r->ipaddr),0)),0);
+ hv_store(rhv, "broadcast", sizeof("broadcast")-1,
sv_2mortal(newSVpv(inet_ntoa(r->bcast),0)),0);
+ hv_store(rhv, "netmask", sizeof("netmask")-1,
sv_2mortal(newSVpv(inet_ntoa(r->netmask),0)),0);

XPUSHs((SV*)sv_2mortal(newRV_inc((SV *)phv)));/* push the psuedo
interface onto the stack */
XPUSHs((SV*)sv_2mortal(newRV_inc((SV *)eav)));/* push the extra
interface array ref onto the stack */


; cvs commit -m "fix for off-by-one in the C->perl layer. perl keys in
the has had the trailing null terminator on them!"
Checking in ChangeLog;
/storage/cvs/munjal/wackamole/ChangeLog,v <-- ChangeLog
new revision: 1.28; previous revision: 1.27
done
Checking in perl.c;
/storage/cvs/munjal/wackamole/perl.c,v <-- perl.c
new revision: 1.5; previous revision: 1.4
done
;


--
// Theo Schlossnagle
// Principal Engineer -- http://www.omniti.com/~jesus/
// Ecelerity: Run with it. -- http://www.omniti.com/


_______________________________________________
wackamole-users mailing list
wackamole-users@lists.backhand.org
http://lists.backhand.org/mailman/listinfo/wackamole-users