Re: libm: avoid left-shifting a negative integer
Date: Wed, 19 Aug 2015 21:25:35 +0200
From: Christian Weisgerber na...@mips.inka.de
I saw this drifting by on FreeBSD (from Dimitry Andric):
In libm's exp2(3), avoid left-shifting a negative integer, which is
undefined. Replace it with the intended value, in a defined way.
OK? Don't bother?
ok kettenis@
Index: src/s_exp2.c
===
RCS file: /cvs/src/lib/libm/src/s_exp2.c,v
retrieving revision 1.7
diff -u -p -r1.7 s_exp2.c
--- src/s_exp2.c 3 Jul 2013 04:46:36 - 1.7
+++ src/s_exp2.c 19 Aug 2015 19:10:38 -
@@ -373,14 +373,14 @@ exp2(double x)
/* Compute r = exp2(y) = exp2t[i0] * p(z - eps[i]). */
t = tbl[i0];/* exp2t[i0] */
z -= tbl[i0 + 1]; /* eps[i0] */
- if (k = -1021 20)
+ if (k = -(1021 20))
INSERT_WORDS(twopk, 0x3ff0 + k, 0);
else
INSERT_WORDS(twopkp1000, 0x3ff0 + k + (1000 20), 0);
r = t + t * z * (P1 + z * (P2 + z * (P3 + z * (P4 + z * P5;
/* Scale by 2**(k20). */
- if(k = -1021 20) {
+ if(k = -(1021 20)) {
if (k == 1024 20)
return (r * 2.0 * 0x1p1023);
return (r * twopk);
--
Christian naddy Weisgerber na...@mips.inka.de
Re: [patch] ctags -u
On Wed, Aug 19, 2015 at 12:53:15PM -0600, Todd C. Miller wrote:
On Wed, 19 Aug 2015 20:21:08 +0200, Sebastien Marie wrote:
Some minor comments inline.
Thanks for the comments.
Here is a new version, which include your comments, and have the
following additional changes:
- do preloading only if -v or -x are absents (same behaviour as
before)
- manage '/' and '?' as possible search char (-F/-B forward/backward
search)
- check for skipping the preloaded entry more early (instead of
parsing pattern and discard it after)
- reflects logic change in ctags.1
One behaviour change remains: if your 'tags' file was previously
generated with '-F' and you update with '-B', the previous behaviour was
to mix /^xxx/ and ?^xxx? in 'tags' file. Here, as the file is
completely regenerated (with preloaded old entries), all the updated
'tags' will be in ?^xxx? format (backward search).
Thanks.
--
Sebastien Marie
Index: ctags.1
===
RCS file: /cvs/src/usr.bin/ctags/ctags.1,v
retrieving revision 1.28
diff -u -p -r1.28 ctags.1
--- ctags.1 13 Mar 2015 19:58:41 - 1.28
+++ ctags.1 20 Aug 2015 10:36:40 -
@@ -89,12 +89,8 @@ The default behaviour is to place them i
Update the specified files in the
.Ar tags
file, that is, all
-references to them are deleted, and the new values are appended to the
+references to them are regenerated, keeping only the others values in the
file.
-(Beware: this option is implemented in a way which is rather
-slow; it is usually faster to simply rebuild the
-.Ar tags
-file.)
.It Fl v
An index of the form expected by vgrind
is produced on the standard output.
Index: ctags.c
===
RCS file: /cvs/src/usr.bin/ctags/ctags.c,v
retrieving revision 1.15
diff -u -p -r1.15 ctags.c
--- ctags.c 8 Feb 2015 23:40:34 - 1.15
+++ ctags.c 20 Aug 2015 10:36:40 -
@@ -65,6 +65,7 @@ char lbuf[LINE_MAX];
void init(void);
void find_entries(char *);
+void preload_entries(char *, int, char *[]);
int
main(int argc, char *argv[])
@@ -75,7 +76,6 @@ main(int argc, char *argv[])
int exit_val; /* exit value */
int step; /* step through args */
int ch; /* getopts char */
- char*cmd;
aflag = uflag = NO;
while ((ch = getopt(argc, argv, BFadf:tuwvx)) != -1)
@@ -122,6 +122,8 @@ usage: (void)fprintf(stderr,
}
init();
+ if (uflag !vflag !xflag)
+ preload_entries(outfile, argc, argv);
for (exit_val = step = 0; step argc; ++step)
if (!(inf = fopen(argv[step], r))) {
@@ -138,30 +140,10 @@ usage:(void)fprintf(stderr,
if (xflag)
put_entries(head);
else {
- if (uflag) {
- for (step = 0; step argc; step++) {
- if (asprintf(cmd,
- mv %s OTAGS; fgrep -v '\t%s\t'
OTAGS %s; rm OTAGS,
- outfile, argv[step], outfile) == -1)
- err(1, out of space);
- system(cmd);
- free(cmd);
- cmd = NULL;
- }
- aflag = 1;
- }
if (!(outf = fopen(outfile, aflag ? a : w)))
err(exit_val, %s, outfile);
put_entries(head);
(void)fclose(outf);
- if (uflag) {
- if (asprintf(cmd, sort -o %s %s,
- outfile, outfile) == -1)
- err(1, out of space);
- system(cmd);
- free(cmd);
- cmd = NULL;
- }
}
}
exit(exit_val);
@@ -252,4 +234,80 @@ find_entries(char *file)
}
}
/* C */c_entries();
+}
+
+void
+preload_entries(char *tagsfile, int argc, char *argv[])
+{
+ FILE*fp;
+ char line[LINE_MAX];
+ char*entry = NULL;
+ char*file = NULL;
+ char*pattern = NULL;
+ char*eol;
+ int i;
+
+ in_preload = YES;
+
+ if ((fp = fopen(tagsfile, r)) == NULL)
+ err(1, preload_entries: %s, tagsfile);
+
+ while (1) {
+next:
+ if (fgets(line, sizeof(line), fp) == NULL)
+ break;
+
+ if ((eol = strchr(line, '\n')) == NULL)
+ errx(1,
Re: ipip interrupt path and list iteration
On 19/08/15(Wed) 20:10, Alexander Bluhm wrote:
On Wed, Aug 19, 2015 at 02:41:22PM +0200, Martin Pieuchot wrote:
Diff below converts one of the few remaining iteration on the global
list of interfaces (ifnet).
/* Check for local address spoofing. */
- if (((ifp = if_get(m-m_pkthdr.ph_ifidx)) == NULL ||
- !(ifp-if_flags IFF_LOOPBACK)) ipip_allow != 2) {
If the packet was coming from a loopback interface it was always
passed. Your code always drops if it is coming from a local address.
I think we need this check, see also the comment mentioning ph_ifidx
above the function ipip_input().
Right, I'm still really interested in an explanation. The comment you
mentioned talks about enc(4) which is not a IFF_LOOPBACK interface. I
just hate these loopback hacks, we have so many of them always checking
for the IFF_LOOPBACK type and I'd rather see fewer if_get(9) usage in
the stack.
Anyway if somebody could comment on that; I'm interested, but I left
it.
+ struct sockaddr_storage ss;
+ struct rtentry *rt;
+
+ if (ipo) {
+ sin = (struct sockaddr_in *)ss;
+ sin-sin_family = AF_INET;
+ sin-sin_len = sizeof(*sin);
+ sin-sin_addr = ipo-ip_src;
+#ifdef INET6
+ } else if (ip6) {
+ sin6 = (struct sockaddr_in6 *)ss;
+ sin6-sin6_family = AF_INET6;
+ sin6-sin6_len = sizeof(*sin6);
+ sin6-sin6_addr = ip6-ip6_src;
#endif /* INET6 */
+ }
Normaly you should memset(0) sin and sin6 because of the padding.
At least in user land and with sobind() this is necessary. Most
of the kernel code does this. But there are a few places where it
is not done before rtalloc(). I think we should initialize it
everywhere. I can build a diff to make it consistent.
Thanks, here's an updated diff.
Index: netinet/ip_ipip.c
===
RCS file: /cvs/src/sys/netinet/ip_ipip.c,v
retrieving revision 1.64
diff -u -p -r1.64 ip_ipip.c
--- netinet/ip_ipip.c 14 Aug 2015 18:07:28 - 1.64
+++ netinet/ip_ipip.c 20 Aug 2015 11:42:26 -
@@ -146,10 +146,8 @@ ipip_input(struct mbuf *m, int iphlen, s
{
struct sockaddr_in *sin;
struct ifnet *ifp;
- struct ifaddr *ifa;
struct niqueue *ifq = NULL;
struct ip *ipo;
- u_int rdomain;
#ifdef INET6
struct sockaddr_in6 *sin6;
struct ip6_hdr *ip6;
@@ -297,43 +295,34 @@ ipip_input(struct mbuf *m, int iphlen, s
/* Check for local address spoofing. */
if (((ifp = if_get(m-m_pkthdr.ph_ifidx)) == NULL ||
!(ifp-if_flags IFF_LOOPBACK)) ipip_allow != 2) {
- rdomain = rtable_l2(m-m_pkthdr.ph_rtableid);
- TAILQ_FOREACH(ifp, ifnet, if_list) {
- if (ifp-if_rdomain != rdomain)
- continue;
- TAILQ_FOREACH(ifa, ifp-if_addrlist, ifa_list) {
- if (ipo) {
- if (ifa-ifa_addr-sa_family !=
- AF_INET)
- continue;
-
- sin = satosin(ifa-ifa_addr);
- if (sin-sin_addr.s_addr ==
- ipo-ip_src.s_addr) {
- ipipstat.ipips_spoof++;
- m_freem(m);
- return;
- }
- }
-#ifdef INET6
- if (ip6) {
- if (ifa-ifa_addr-sa_family !=
- AF_INET6)
- continue;
-
- sin6 = satosin6(ifa-ifa_addr);
- if (IN6_ARE_ADDR_EQUAL(sin6-sin6_addr,
- ip6-ip6_src)) {
- ipipstat.ipips_spoof++;
- m_freem(m);
- return;
- }
+ struct sockaddr_storage ss;
+ struct rtentry *rt;
- }
+ memset(ss, 0, sizeof(ss));
+
+ if (ipo) {
+ sin = (struct sockaddr_in *)ss;
+ sin-sin_family = AF_INET;
+ sin-sin_len = sizeof(*sin);
+ sin-sin_addr = ipo-ip_src;
+#ifdef INET6
+ } else if (ip6) {
+ sin6 = (struct sockaddr_in6 *)ss;
+
Re: Fwd: worm.c removing unused variables
Rafael Zalamena wrote: On Mon, Aug 17, 2015 at 01:00:26PM -0300, Vinicios Barros wrote: Hello all, I would like to suggest these changes to remove unused variables and a respectively unnecessary call of the gettimeofday, also removes a casting in the malloc, that seems to be unnecessary. I improved your diff a little bit to clean up more things. * Kill some unused includes; * Call poll() with time parameter direcly instead of doing some mathematical operations; * Kill commented fflush() code; I'll leave the malloc() bit to another diff, since it might be better and safer to replace it with a calloc(). Btw your diff wouldn't apply because your mail client fucked the spaces up. any oks? ok
Re: RTM_DELETE and route refcount
On 20/08/15(Thu) 18:20, Mike Belopuhov wrote:
Makes you wonder why the heck it wasn't done in the first place,
doesn't it?
If you look at the original CSRG source tree, you'll see how/why
this happened :)
When karels@ changed rtrequest() to turn it into a frontend for the
radix tree in r7.8 of net/route.c RTM_DELETE was not returning a rt.
At this point rt_refcnt was not that used.
All this nightmare really started with the introduction of cloning
routes. Until today you can see the mess in rtalloc(9) to increment
the cloned vs cloning route depending on what happened. This was
r7.13 of net/route.c and in this commit a call to rtrequest(RTM_DELETE)
would first rtfree(9) the route given in argument *then* try to remove
it from the routing table.
Later in r7.23 rtrequest(RTM_DELETE) stopped to free the given route and
rtinit() (what's today rt_ifa_del(9)) started to rtfree(9) the route
itself always before removing it from the tree.
Obviously this couldn't work and in r7.29 sklower@ made rtrequest()
return the route in the RTM_DELETE case to be able to free it. At this
point rt_refcnt was *not* incremented prior to rfree(9) a route. Note
that a valid route in the tree without external reference has a count
of 0. So in order to actually free the route the following pattern has
been copy/pasted from rtrequest():
if (rt-rt_refcnt = 0)
rtfree(rt);
Finally in r7.31 this idiom has been changed to:
if (rt-rt_refcnt = 0) {
rt-rt_refcnt++;
rtfree(rt);
}
I guess as an easy fix because the panic() in rtfree(9) triggered when
rtalloc(9) returned a cloning route instead of a cloned one without
incrementing it's reference counter.
Then this pattern has been copy/pasted everywhere, like most of the good
and not so good stuff we find in this tree.
All of that happened between 1988 and 1992, because of a bug in the
reference count of RTF_CLONING routes, obviously we weren't their to
do peer review ;)
That's why I appreciate as much review as possible in this area, I'm
sure we will have (or already do) similar issues.
Re: v6 autoconf, where have my connected routes gone?
On 08/19/15 17:51, Martin Pieuchot wrote:
On 18/08/15(Tue) 23:22, Martin Pieuchot wrote:
On 18/08/15(Tue) 14:00, Stuart Henderson wrote:
On 2015/08/18 14:27, Martin Pieuchot wrote:
On 18/08/15(Tue) 13:05, Stuart Henderson wrote:
I'm trying to add v6 to my second ISP connection on pppoe1, unlike my
first ISP, this one requires autoconf and dhcpv6-pd to pick up addresses
and have them routed. (They are referring to TR-187,
https://www.broadband-forum.org/technical/download/TR-187.pdf -
the residential gateway requirements are in section 6.1)
I haven't got as far as looking at dhcpv6-pd because when I enable
autoconf, as soon as it picks up the address I lose connected routes
to my local /64 subnets:
Which prefixes (ndp -p) and default routers (ndp -r) do you have before
and after the interface picks up the address?
Before: no default routers showing with ndp -r.
$ ndp -p | paste - - - | sed 's/vltime=infinity, pltime=infinity, expire=Never,
//'
[...]
2001:4b10:1002:cc01::/64 if=vlan2 flags=LO ref=1No advertising router
... After:
$ ndp -r
[...]
2001:4b10:1002:cc01::/64 if=vlan2 flags=LD ref=1No advertising router
See how all your prefix suddenly became detached D? That's a
limitation of pfxlist_onlink_check() which does not make a difference
between prefixes added to the global prefix list via an advertisement
or via manual configuration.
I'll see if it's possible to split the prefix lists in such way that
the RS/RA code only mess with its own states.
Diff below is just slightly tested and depends on the diff to
Kill IN6_IFF_NODAD but it shows where I'd like to go.
The idea is to use the global prefix list only for AUTOCONF'd addresses.
Since this list is only checked in nd6_is_addr_neighbor() I added a new
case for non-AUTOCONF'd addresses. With it you should not longer see
the prefix of your manually configured interfaces in the ndp -p output
but it's ok since they should always be in your routing table.
That problem sthen@ describes here, sounds very similar to my home situation.
My router has IPv6 /48 HE tunnel, and on the wired network, i have /64 subnet
where all hosts have statically configured IPv6 addresses, incl. default route.
The router is also WLAN AP, where it has rtadvd running on the WLAN if,
and announcing a different /64 subnet.
Notebooks and other gear, that might have WLAN and network interfaces, when
connected to the wired network, and autoconf on the WLAN interface kicked in,
the IPv6 connection to the local /64 network were gone.
With the patch below, together with the other patch you mention above,
my problem is gone, network works as I expect it.
cheers,
Sebastian
diff --git sys/netinet6/in6.c sys/netinet6/in6.c
index 2f5d5a6..e9101dc 100644
--- sys/netinet6/in6.c
+++ sys/netinet6/in6.c
@@ -462,7 +462,6 @@ in6_control(struct socket *so, u_long cmd, caddr_t data,
struct ifnet *ifp)
case SIOCAIFADDR_IN6:
{
- struct nd_prefix *pr;
int plen, error = 0;
/* reject read-only flags */
@@ -509,40 +508,19 @@ in6_control(struct socket *so, u_long cmd, caddr_t data,
struct ifnet *ifp)
if (ia6-ia6_flags IN6_IFF_TENTATIVE)
nd6_dad_start(ia6-ia_ifa);
-
plen = in6_mask2len(ifra-ifra_prefixmask.sin6_addr, NULL);
if (plen == 128) {
dohooks(ifp-if_addrhooks, 0);
break; /* we don't need to install a host route. */
}
- /*
-* then, make the prefix on-link on the interface.
-* XXX: we'd rather create the prefix before the address, but
-* we need at least one address to install the corresponding
-* interface route, so we configure the address first.
-*/
- pr = nd6_prefix_add(ifp, ifra-ifra_addr,
- ifra-ifra_prefixmask, ifra-ifra_lifetime,
- ((ifra-ifra_flags IN6_IFF_AUTOCONF) != 0));
- if (pr == NULL) {
- log(LOG_ERR, cannot add prefix\n);
- return (EINVAL); /* XXX panic here? */
- }
-
- /* relate the address to the prefix */
- if (ia6-ia6_ndpr == NULL) {
- ia6-ia6_ndpr = pr;
- pr-ndpr_refcnt++;
+ error = rt_ifa_add(ia6-ia_ifa,
+ RTF_UP|RTF_CLONING|RTF_CONNECTED, ia6-ia_ifa.ifa_addr);
+ if (error) {
+ in6_purgeaddr(ia6-ia_ifa);
+ return (error);
}
-
s = splsoftnet();
- /*
-* this might affect the status of autoconfigured addresses,
-* that is, this address might make other addresses detached.
-*/
- pfxlist_onlink_check();
-
dohooks(ifp-if_addrhooks, 0);
Fix ndp -p unaligned access (kernel and userland)
Currently, ndp -p will trigger unaligned accesses in the kernel and
userland on architectures that require strict alignment for 64-bit
types (e.g. sparc64).
This is actually fallout from the time_t change.
The sysctl used to pass the prefix list uses a packed format:
one struct in6_prefix, several struct sockaddr_in6,
one struct in6_prefix, several struct sockaddr_in6,
...
This worked dandily when both structs had the same alignment
requirement (int32_t). However, there's a time_t in in6_prefix
which pushed that struct's alignment to int64_t... boom!
The patches below are taken from NetBSD (Martin Husemann):
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/nd6.c.diff?r1=1.145r2=1.146
http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/ndp/ndp.c.diff?r1=1.41r2=1.42
Use simple byte pointer arithmetic and memcpy from/to aligned stack
variables to handle the packed binary format passed out to userland
when querying the prefix/router list.
OK?
Index: sys/netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.143
diff -u -p -r1.143 nd6.c
--- sys/netinet6/nd6.c 16 Jul 2015 15:31:35 - 1.143
+++ sys/netinet6/nd6.c 20 Aug 2015 14:36:19 -
@@ -1877,45 +1877,43 @@ fill_prlist(void *oldp, size_t *oldlenp,
{
int error = 0, s;
struct nd_prefix *pr;
- struct in6_prefix *p = NULL;
- struct in6_prefix *pe = NULL;
+ char *p = NULL, *ps = NULL;
+ char *pe = NULL;
size_t l;
s = splsoftnet();
if (oldp) {
- p = (struct in6_prefix *)oldp;
- pe = (struct in6_prefix *)((caddr_t)oldp + *oldlenp);
+ ps = p = (char *)oldp;
+ pe = (char *)oldp + *oldlenp;
}
l = 0;
LIST_FOREACH(pr, nd_prefix, ndpr_entry) {
u_short advrtrs;
- size_t advance;
- struct sockaddr_in6 *sin6;
- struct sockaddr_in6 *s6;
+ struct sockaddr_in6 sin6;
struct nd_pfxrouter *pfr;
+ struct in6_prefix pfx;
char addr[INET6_ADDRSTRLEN];
- if (oldp p + 1 = pe)
- {
- bzero(p, sizeof(*p));
- sin6 = (struct sockaddr_in6 *)(p + 1);
-
- p-prefix = pr-ndpr_prefix;
- if (in6_recoverscope(p-prefix,
- p-prefix.sin6_addr, pr-ndpr_ifp) != 0)
+ if (oldp p + sizeof(struct in6_prefix) = pe) {
+ memset(pfx, 0, sizeof(pfx));
+ ps = p;
+
+ pfx.prefix = pr-ndpr_prefix;
+ if (in6_recoverscope(pfx.prefix,
+ pfx.prefix.sin6_addr, pr-ndpr_ifp) != 0)
log(LOG_ERR,
scope error in prefix list (%s)\n,
- inet_ntop(AF_INET6, p-prefix.sin6_addr,
+ inet_ntop(AF_INET6, pfx.prefix.sin6_addr,
addr, sizeof(addr)));
- p-raflags = pr-ndpr_raf;
- p-prefixlen = pr-ndpr_plen;
- p-vltime = pr-ndpr_vltime;
- p-pltime = pr-ndpr_pltime;
- p-if_index = pr-ndpr_ifp-if_index;
+ pfx.raflags = pr-ndpr_raf;
+ pfx.prefixlen = pr-ndpr_plen;
+ pfx.vltime = pr-ndpr_vltime;
+ pfx.pltime = pr-ndpr_pltime;
+ pfx.if_index = pr-ndpr_ifp-if_index;
if (pr-ndpr_vltime == ND6_INFINITE_LIFETIME)
- p-expire = 0;
+ pfx.expire = 0;
else {
time_t maxexpire;
@@ -1924,40 +1922,44 @@ fill_prlist(void *oldp, size_t *oldlenp,
((sizeof(maxexpire) * 8) - 1));
if (pr-ndpr_vltime
maxexpire - pr-ndpr_lastupdate) {
- p-expire = pr-ndpr_lastupdate +
+ pfx.expire = pr-ndpr_lastupdate +
pr-ndpr_vltime;
} else
- p-expire = maxexpire;
+ pfx.expire = maxexpire;
}
- p-refcnt = pr-ndpr_refcnt;
- p-flags = pr-ndpr_stateflags;
- p-origin = PR_ORIG_RA;
+ pfx.refcnt = pr-ndpr_refcnt;
+ pfx.flags = pr-ndpr_stateflags;
+ pfx.origin = PR_ORIG_RA;
+
+ p += sizeof(pfx); l +=
Re: RTM_DELETE and route refcount
Makes you wonder why the heck it wasn't done in the first place, doesn't it?
