ospf6d: sync database.c with ospfd(8)
This is mostly log messages sync.
Index: database.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/database.c,v
retrieving revision 1.18
diff -u -p -r1.18 database.c
--- database.c 23 Dec 2019 07:33:49 - 1.18
+++ database.c 2 Jan 2020 14:31:46 -
@@ -43,7 +43,6 @@ send_db_description(struct nbr *nbr)
struct db_dscrp_hdr dd_hdr;
struct lsa_entry*le, *nle;
struct ibuf *buf;
- int ret = 0;
u_int8_t bits = 0;
if ((buf = ibuf_open(nbr->iface->mtu - sizeof(struct ip6_hdr))) == NULL)
@@ -63,11 +62,10 @@ send_db_description(struct nbr *nbr)
case NBR_STA_INIT:
case NBR_STA_2_WAY:
case NBR_STA_SNAP:
- log_debug("send_db_description: cannot send packet in state %s,"
- " neighbor ID %s", nbr_state_name(nbr->state),
- inet_ntoa(nbr->id));
- ret = -1;
- goto done;
+ log_debug("send_db_description: neighbor ID %s: "
+ "cannot send packet in state %s", inet_ntoa(nbr->id),
+ nbr_state_name(nbr->state));
+ goto fail;
case NBR_STA_XSTRT:
bits |= OSPF_DBD_MS | OSPF_DBD_M | OSPF_DBD_I;
nbr->dd_more = 1;
@@ -90,7 +88,7 @@ send_db_description(struct nbr *nbr)
/* build LSA list */
for (le = TAILQ_FIRST(>db_sum_list); le != NULL &&
- buf->wpos + sizeof(struct lsa_hdr) < buf->max; le = nle) {
+ ibuf_left(buf) >= sizeof(struct lsa_hdr); le = nle) {
nbr->dd_end = nle = TAILQ_NEXT(le, entry);
if (ibuf_add(buf, le->le_lsa, sizeof(struct lsa_hdr)))
goto fail;
@@ -146,10 +144,11 @@ send_db_description(struct nbr *nbr)
goto fail;
/* transmit packet */
- ret = send_packet(nbr->iface, buf, );
-done:
+ if (send_packet(nbr->iface, buf, ) == -1)
+ goto fail;
+
ibuf_free(buf);
- return (ret);
+ return (0);
fail:
log_warn("send_db_description");
ibuf_free(buf);
@@ -163,8 +162,8 @@ recv_db_description(struct nbr *nbr, cha
int dupe = 0;
if (len < sizeof(dd_hdr)) {
- log_warnx("recv_db_description: "
- "bad packet size, neighbor ID %s", inet_ntoa(nbr->id));
+ log_warnx("recv_db_description: neighbor ID %s: "
+ "bad packet size", inet_ntoa(nbr->id));
return;
}
memcpy(_hdr, buf, sizeof(dd_hdr));
@@ -173,9 +172,9 @@ recv_db_description(struct nbr *nbr, cha
/* db description packet sanity checks */
if (ntohs(dd_hdr.iface_mtu) > nbr->iface->mtu) {
- log_warnx("recv_db_description: invalid MTU %d sent by "
- "neighbor ID %s, expected %d", ntohs(dd_hdr.iface_mtu),
- inet_ntoa(nbr->id), nbr->iface->mtu);
+ log_warnx("recv_db_description: neighbor ID %s: "
+ "invalid MTU %d expected %d", inet_ntoa(nbr->id),
+ ntohs(dd_hdr.iface_mtu), nbr->iface->mtu);
return;
}
@@ -183,7 +182,7 @@ recv_db_description(struct nbr *nbr, cha
nbr->last_rx_bits == dd_hdr.bits &&
ntohl(dd_hdr.dd_seq_num) == nbr->dd_seq_num - nbr->dd_master ?
1 : 0) {
- log_debug("recv_db_description: dupe from ID %s",
+ log_debug("recv_db_description: dupe from neighbor ID %s",
inet_ntoa(nbr->id));
dupe = 1;
}
@@ -193,9 +192,9 @@ recv_db_description(struct nbr *nbr, cha
case NBR_STA_ATTEMPT:
case NBR_STA_2_WAY:
case NBR_STA_SNAP:
- log_debug("recv_db_description: packet ignored in state %s, "
- "neighbor ID %s", nbr_state_name(nbr->state),
- inet_ntoa(nbr->id));
+ log_debug("recv_db_description: neighbor ID %s: "
+ "packet ignored in state %s", inet_ntoa(nbr->id),
+ nbr_state_name(nbr->state));
return;
case NBR_STA_INIT:
/* evaluate dr and bdr after issuing a 2-Way event */
@@ -224,9 +223,11 @@ recv_db_description(struct nbr *nbr, cha
} else if (!(dd_hdr.bits & (OSPF_DBD_I | OSPF_DBD_MS))) {
/* M only case: we are master */
if (ntohl(dd_hdr.dd_seq_num) != nbr->dd_seq_num) {
- log_warnx("recv_db_description: invalid "
- "seq num, mine %x his %x",
- nbr->dd_seq_num, ntohl(dd_hdr.dd_seq_num));
+ log_warnx("recv_db_description: "
+
vmctl(8): uninitialized value
Hi. "case CMD_SEND:" sets done=1 so ret will never be written to and the uninitialized value of ret is used to determine the return value of the function vmmaction. -- Ben Index: main.c === RCS file: /cvs/src/usr.sbin/vmctl/main.c,v retrieving revision 1.61 diff -u -p -r1.61 main.c --- main.c 28 Dec 2019 18:36:01 - 1.61 +++ main.c 2 Jan 2020 15:09:28 - @@ -265,6 +265,7 @@ vmmaction(struct parse_result *res) case CMD_SEND: send_vm(res->id, res->name); done = 1; + ret = 0; break; case CMD_RECEIVE: vm_receive(res->id, res->name);
Re: snmpd(8): filter pf table addresses
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.12.31 11:44:07 +0100: > On Tue, Dec 31, 2019 at 11:16:37AM +0100, Martijn van Duren wrote: > > I'm on the fence about this. So if you feel strongly about this go > > ahead if it works. > > In some regard I agree but in this case I think it makes sense. I think it even is useful if snmpd were more efficient in retrieving the data from pf. It just is a waste of cpu and bandwith to transfer these is you know in advance that you dont need them. > > I am however somewhat confused about your description. You say it > > times out, but considering it (by default) only retrieves 10 entries > > per packet it should return somewhat quick and not cause a timeout. > > If you want to move through it quicker you can increase the amount > > retrieved per request by setting -CrX where X can be any value. > > I reckon -Cr40 is a decent number. > > I doubt this will help. Looking at the code it seems that snmpd will fetch > the full pftable from the kernel for each request which is probably where > the inefficency and timeout happen. Also it does a linear search over > Florian's 300'000 addresses to pick up the next address. > > In short the pftable code and probably also the rest of the pf code in > snmpd is not built to scale. There is no caching, no fast lookups, no > smart paging implemented. This code needs major work to be usable. > > I think the diff by Florian is OK and should go in. +1 ok benno@
ospf6d: sync hello.c with ospfd
Sync with ospfd's hello.c
Index: hello.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/hello.c,v
retrieving revision 1.21
diff -u -p -r1.21 hello.c
--- hello.c 23 Dec 2019 11:25:41 - 1.21
+++ hello.c 2 Jan 2020 16:11:19 -
@@ -41,8 +41,6 @@ send_hello(struct iface *iface)
struct hello_hdr hello;
struct nbr *nbr;
struct ibuf *buf;
- int ret;
- u_int32_topts;
switch (iface->type) {
case IF_TYPE_POINTOPOINT:
@@ -72,10 +70,8 @@ send_hello(struct iface *iface)
/* hello header */
hello.iface_id = htonl(iface->ifindex);
LSA_24_SETHI(hello.opts, iface->priority);
- opts = area_ospf_options(iface->area);
- LSA_24_SETLO(hello.opts, opts);
+ LSA_24_SETLO(hello.opts, area_ospf_options(iface->area));
hello.opts = htonl(hello.opts);
-
hello.hello_interval = htons(iface->hello_interval);
hello.rtr_dead_interval = htons(iface->dead_interval);
@@ -104,10 +100,11 @@ send_hello(struct iface *iface)
if (upd_ospf_hdr(buf, iface))
goto fail;
- ret = send_packet(iface, buf, );
+ if (send_packet(iface, buf, ) == -1)
+ goto fail;
ibuf_free(buf);
- return (ret);
+ return (0);
fail:
log_warn("send_hello");
ibuf_free(buf);
@@ -120,7 +117,6 @@ recv_hello(struct iface *iface, struct i
{
struct hello_hdr hello;
struct nbr *nbr = NULL, *dr;
- struct area *area;
u_int32_tnbr_id, opts;
int nbr_change = 0;
@@ -148,12 +144,9 @@ recv_hello(struct iface *iface, struct i
return;
}
- if ((area = iface->area) == NULL)
- fatalx("interface lost area");
-
opts = LSA_24_GETLO(ntohl(hello.opts));
- if ((opts & OSPF_OPTION_E && area->stub) ||
- ((opts & OSPF_OPTION_E) == 0 && !area->stub)) {
+ if ((opts & OSPF_OPTION_E && iface->area->stub) ||
+ ((opts & OSPF_OPTION_E) == 0 && !iface->area->stub)) {
log_warnx("recv_hello: ExternalRoutingCapability mismatch, "
"interface %s", iface->name);
return;
@@ -161,8 +154,15 @@ recv_hello(struct iface *iface, struct i
/* match router-id */
LIST_FOREACH(nbr, >nbr_list, entry) {
- if (nbr == iface->self)
+ if (nbr == iface->self) {
+ if (nbr->id.s_addr == rtr_id) {
+ log_warnx("recv_hello: Router-ID collision on "
+ "interface %s neighbor IP %s", iface->name,
+ log_in6addr(src));
+ return;
+ }
continue;
+ }
if (nbr->id.s_addr == rtr_id)
break;
}
Re: Add -R alias to -r for scp(1)
Hi Marc, Marc Espie wrote on Thu, Jan 02, 2020 at 11:30:35AM +0100: > And if I use scp enough, I'm also likely to use cp -r by mistake. > > Are we likely to actually remove cp -r so the second mistake > doesn't happen ? I wouldn't be opposed to that. It has been deprecated since rev. 1.1 in 1995. FreeBSD also deprecates it but has subtly different behaviour. NetBSD has the same deprecation notice we have. In GNU cp(1), according to the manual, it seems to be an alias for cp -R. Illumos and Oracle Solaris seem to somewhat resemble FreeBSD - not sure all is identical - but -r is not deprecated. Given these differences, it seems doubtful how much sense it makes to keep it "for compatibility". However, deleting it would require a full make build and a ports bulk build, i guess. If it turns out it sees substantial use in the wild, i think we should make cp -r a deprecated alias for cp -R because i expect that almost every software out there using it (if any) probably comes from the Linux world, given that is has been deprecated in all BSDs for more than two decades. Yours, Ingo
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
> > On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
> >
> > I'd like to stop converting the given timespec to ticks and instead use
> > nanoseconds. This is part of the ongoing effort to reduce the use of
> > `hz' through the kernel.
> >
> > Since I don't know C I'd appreciate any pointer about the checks that
> > should be added to TIMESPEC_TO_NSEC().
> >
> > Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
>
> We can't do this until timeouts have a tickless interface. Otherwise
> your timeouts will return early. That's why I was saving the sys/kern
> conversions until after resolving that issue.
I don't understand, can you elaborate?
Re: vmctl(8): uninitialized value
On Thu, Jan 02, 2020 at 04:37:17PM +0100, Benjamin Baier wrote: > "case CMD_SEND:" sets done=1 so ret will never be written to and > the uninitialized value of ret is used to determine the return > value of the function vmmaction. Good catch: $ doas vmctl start -b ~/vm/bsd.rd -m 128M test ; echo $? vmctl: starting without disks vmctl: starting without network interfaces vmctl: started vm 4 successfully, tty /dev/ttyp2 0 $ doas vmctl send test >/dev/null ; echo $? vmctl: sent vm test successfully 1 With your diff it exits zero. I also just noticed that above example reproducibly causes vmd(8) to exit: Jan 2 18:53:57 eru vmd[55128]: startup Jan 2 18:54:18 eru vmd[55128]: test: started vm 4 successfully, tty /dev/ttyp2 Jan 2 18:54:28 eru vmd[49983]: priv exiting, pid 49983 Jan 2 18:54:28 eru vmd[29885]: control exiting, pid 29885 OK kn if anyone wants to commit, otherwise I'll do so tomorrow when looking into this issue.
Re: sparc64: find root device on hardware RAID
On Tue, Dec 31, 2019 at 09:12:56AM +1000, Jonathan Matthew wrote: > I think this is probably the most sensible thing we can do here. > ok jmatthew@ but I'd wait and see if anyone has a better idea. I'll commit later this evening so that snapshots will just work on my machine, this makes upgrading easier for me. We can still improve this in-tree if someone comes up with a better idea.
Re: FAQ4.html Commit Revert
For me, faq4.html displays fine in Firefox, lynx, and Chromium, both with and without the CSS. On Thu, Jan 2, 2020 at 10:47 AM Oleg Pahl wrote: > > Hi @All, > > > > could you be so kind to revert this commit in FAQ 4? > > https://cvsweb.openbsd.org/cgi-bin/cvsweb/www/faq/faq4.html.diff?r1=1.495=1.496 > > > Commit message : convert two ugly bullet lists into definition lists because > that's what they are. > ok tj > --- > > > > Before > > > After > > > BR, > > Oleg Pahl
Re: vmctl(8): uninitialized value
* Benjamin Baier [2020-01-02 22:01:14 +0100]: On Thu, 2 Jan 2020 18:56:14 +0100 Klemens Nanni wrote: On Thu, Jan 02, 2020 at 04:37:17PM +0100, Benjamin Baier wrote: > "case CMD_SEND:" sets done=1 so ret will never be written to and > the uninitialized value of ret is used to determine the return > value of the function vmmaction. Good catch: $ doas vmctl start -b ~/vm/bsd.rd -m 128M test ; echo $? vmctl: starting without disks vmctl: starting without network interfaces vmctl: started vm 4 successfully, tty /dev/ttyp2 0 $ doas vmctl send test >/dev/null ; echo $? vmctl: sent vm test successfully 1 With your diff it exits zero. I also just noticed that above example reproducibly causes vmd(8) to exit: Jan 2 18:53:57 eru vmd[55128]: startup Jan 2 18:54:18 eru vmd[55128]: test: started vm 4 successfully, tty /dev/ttyp2 Jan 2 18:54:28 eru vmd[49983]: priv exiting, pid 49983 Jan 2 18:54:28 eru vmd[29885]: control exiting, pid 29885 I don't get vmd to exit, but got this in /var/log/messages with the above example: Jan 2 21:38:05 x220 vmd[86810]: control_dispatch_vmd: lost control connection: fd 7 Hi Benjamin, kn@ is running with vm.malloc_conf=SU. The crash is due to vm_remove calling free on vm here, https://github.com/openbsd/src/blob/926f477f07d3bba063ff6ee1ea9e0b7369ed8930/usr.sbin/vmd/vmm.c#L542 will work on a fix. -- Pratik
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
> On Jan 2, 2020, at 9:02 AM, Martin Pieuchot wrote:
>
> On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
>>> On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
>>>
>>> I'd like to stop converting the given timespec to ticks and instead use
>>> nanoseconds. This is part of the ongoing effort to reduce the use of
>>> `hz' through the kernel.
>>>
>>> Since I don't know C I'd appreciate any pointer about the checks that
>>> should be added to TIMESPEC_TO_NSEC().
>>>
>>> Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
>>
>> We can't do this until timeouts have a tickless interface. Otherwise
>> your timeouts will return early. That's why I was saving the sys/kern
>> conversions until after resolving that issue.
>
> I don't understand, can you elaborate?
Timeout are scheduled against the current value of "ticks". Any time that
has elapsed since the current tick began is unaccounted for. You need to
add a tick to your sleep to account for it. tstohz(9) does this. We don't
do it automatically for the *sleep_nsec(9) interfaces because that would
have complicated the conversions we're doing and probably broken callers
before we were ready to break them.
When we have a tickless interface for the timeout(9) layer you will be
able to set a timeout to expire at a given absolute uptime with nanosecond
precision. Then you won't need to work around ticks. It'll be based on
the actual system uptime, not "ticks", which is too coarse an approximation.
With the aforementioned interface we'll be able to write a *sleep(9)
interface that takes advantage of it. And then we can convert the
callers in sys/kern.
Re: vmctl(8): uninitialized value
On Thu, 2 Jan 2020 18:56:14 +0100 Klemens Nanni wrote: > On Thu, Jan 02, 2020 at 04:37:17PM +0100, Benjamin Baier wrote: > > "case CMD_SEND:" sets done=1 so ret will never be written to and > > the uninitialized value of ret is used to determine the return > > value of the function vmmaction. > Good catch: > > $ doas vmctl start -b ~/vm/bsd.rd -m 128M test ; echo $? > vmctl: starting without disks > vmctl: starting without network interfaces > vmctl: started vm 4 successfully, tty /dev/ttyp2 > 0 > $ doas vmctl send test >/dev/null ; echo $? > vmctl: sent vm test successfully > 1 > > With your diff it exits zero. > > I also just noticed that above example reproducibly causes vmd(8) to > exit: > > Jan 2 18:53:57 eru vmd[55128]: startup > Jan 2 18:54:18 eru vmd[55128]: test: started vm 4 successfully, tty > /dev/ttyp2 > Jan 2 18:54:28 eru vmd[49983]: priv exiting, pid 49983 > Jan 2 18:54:28 eru vmd[29885]: control exiting, pid 29885 I don't get vmd to exit, but got this in /var/log/messages with the above example: Jan 2 21:38:05 x220 vmd[86810]: control_dispatch_vmd: lost control connection: fd 7 root# vmd -dvvv startup vm_register: registering vm 1 /etc/vm.conf:12: vm "amd64" registered (disabled) /etc/vm.conf:16: switch "uplink" registered vm_priv_brconfig: interface bridge0 description switch1-uplink vmd_configure: setting staggered start configuration to parallelism: 4 and delay: 30 vmd_configure: starting vms in staggered fashion start_vm_batch: starting batch of 4 vms start_vm_batch: not starting vm amd64 (disabled) start_vm_batch: done starting vms config_getconfig: priv retrieving config config_getconfig: control retrieving config config_getconfig: vmm retrieving config vm_register: registering vm 2 vm_opentty: vm test tty /dev/ttyp8 uid 0 gid 4 mode 620 vm_register: registering vm 2 test: started vm 2 successfully, tty /dev/ttyp8 loadfile_elf: loaded ELF kernel run_vm: initializing hardware for vm test pic_set_elcr: setting level triggered mode for irq 3 pic_set_elcr: setting level triggered mode for irq 5 run_vm: starting vcpu threads for vm test vcpu_reset: resetting vcpu 0 for vm 8 run_vm: waiting on events for VM test vcpu_exit_i8253: channel 0 reset, mode=2, start=65535 vcpu_process_com_lcr: set baudrate = 115200 i8259_write_datareg: master pic, reset IRQ vector to 0x20 i8259_write_datareg: slave pic, reset IRQ vector to 0x28 vcpu_exit_i8253: channel 0 reset, mode=2, start=11932 vcpu_process_com_lcr: set baudrate = 115200 vcpu_process_com_lcr: set baudrate = 115200 vmd_dispatch_vmm: running vm: 2, vm_state: 0x1 vmd_dispatch_vmm: vm: 1, vm_state: 0x2 vmd_dispatch_control: sending fd to vmm i8253_dump: sending PIT i8259_dump: sending PIC i8259_dump: sending ELCR ns8250_dump: sending UART mc146818_dump: sending RTC fw_cfg_dump: sending fw_cfg state pci_dump: sending pci viornd_dump: sending viornd vioblk_dump: sending vioblk vionet_dump: sending vionet vmmci_dump: sending vmmci vm_remove: vmm vmm_dispatch_vm removing vm 2 from running config vm_stop: vmm vmm_dispatch_vm stopping vm 2 test: sent vm 2 successfully. vm_remove: parent vmd_dispatch_vmm removing vm 2 from running config vm_stop: parent vmd_dispatch_vmm stopping vm 2 control_dispatch_vmd: lost control connection: fd 7 vmm_sighdlr: handling signal 20 > OK kn if anyone wants to commit, otherwise I'll do so tomorrow when > looking into this issue. >
Re: PATCH: fix race condition in binutils-2.17 build
On Thu, Jan 2, 2020 at 2:26 AM Marc Espie wrote: > We don't normally hit this thanks to the "expensive job" heuristics > (meaning that we don't start the second target while the first is > building), but still there is a race, and if for whatever reason we > remove that heuristics we hit it fairly hard. > > The patch is fairly trivial, just run both targets separately. > Example of a crash and analysis follows. > > Note that the problem only occurs in binutils-2.17. The former versions > did have one less level of indirection in their Makefiles. > ok guenther@
Re: Add -R alias to -r for scp(1)
Hi Kurt,
Kurt Mosiejczuk wrote on Wed, Jan 01, 2020 at 08:21:04PM -0500:
> cp(1) uses -R for recursive copy. scp(1) uses -r. This diff adds -R
> as an alias for -r to scp(1) for those assuming consistency with cp(1).
even if cp -R and scp -r did the same thing - which, if i understand
tedu@ correctly, is not the case - i would still dislike introducing
an alias.
Aliases make documentation longer for no benefit and sometimes
also cause confusion. They also block namespace for no benefit
(or rather, option space in this case).
While it is nice when different commands can use the same options
for similar purposes, that is not always possible. And even when
it would have been possible when designing the tools in the past,
if a different decision was made back then, remaining consistent
with previous versions of the same command is usually worth more
than symmetry with a different command.
Sometimes, aliases are needed because different other operating
systems use different names and we want to be compatible with both.
But whenever they can be avoided, they should.
When working on documentation, i'm actively trying to deprecate
aliases where that is possible.
Yours,
Ingo
> Index: scp.1
> ===
> RCS file: /cvs/src/usr.bin/ssh/scp.1,v
> retrieving revision 1.87
> diff -u -p -r1.87 scp.1
> --- scp.1 30 Nov 2019 07:07:59 - 1.87
> +++ scp.1 2 Jan 2020 01:08:01 -
> @@ -215,6 +215,12 @@ Recursively copy entire directories.
> Note that
> .Nm
> follows symbolic links encountered in the tree traversal.
> +.It Fl R
> +A synonym for
> +.Fl r .
> +(To mimic the
> +.Xr cp 1
> +flag for recursive copy).
> .It Fl S Ar program
> Name of
> .Ar program
> Index: scp.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/scp.c,v
> retrieving revision 1.206
> diff -u -p -r1.206 scp.c
> --- scp.c 9 Sep 2019 02:31:19 - 1.206
> +++ scp.c 2 Jan 2020 01:08:01 -
> @@ -404,7 +404,7 @@ main(int argc, char **argv)
>
> fflag = Tflag = tflag = 0;
> while ((ch = getopt(argc, argv,
> - "dfl:prtTvBCc:i:P:q12346S:o:F:J:")) != -1) {
> + "dfl:pRrtTvBCc:i:P:q12346S:o:F:J:")) != -1) {
> switch (ch) {
> /* User-visible flags. */
> case '1':
> @@ -452,6 +452,7 @@ main(int argc, char **argv)
> case 'p':
> pflag = 1;
> break;
> + case 'R':
> case 'r':
> iamrecursive = 1;
> break;
Re: Add -R alias to -r for scp(1)
On Thu, Jan 02, 2020 at 11:19:30AM +0100, Ingo Schwarze wrote: > Hi Kurt, > > Kurt Mosiejczuk wrote on Wed, Jan 01, 2020 at 08:21:04PM -0500: > > > cp(1) uses -R for recursive copy. scp(1) uses -r. This diff adds -R > > as an alias for -r to scp(1) for those assuming consistency with cp(1). > > even if cp -R and scp -r did the same thing - which, if i understand > tedu@ correctly, is not the case - i would still dislike introducing > an alias. > > Aliases make documentation longer for no benefit and sometimes > also cause confusion. They also block namespace for no benefit > (or rather, option space in this case). > > While it is nice when different commands can use the same options > for similar purposes, that is not always possible. And even when > it would have been possible when designing the tools in the past, > if a different decision was made back then, remaining consistent > with previous versions of the same command is usually worth more > than symmetry with a different command. > > Sometimes, aliases are needed because different other operating > systems use different names and we want to be compatible with both. > But whenever they can be avoided, they should. > > When working on documentation, i'm actively trying to deprecate > aliases where that is possible. > > Yours, > Ingo Contrary point: I'm with Kurt on that one. Very often, I do mix up -r and -R, and I do hate it. Especially since our cp *does* have a -r which is subtly different. Not having -R in scp means you can do all kinds of fuckups. Once in three times, I type scp -R and go "oh fuck" when it doesn't work. And if I use scp enough, I'm also likely to use cp -r by mistake. Are we likely to actually remove cp -r so the second mistake doesn't happen ?
Re: ospf6d: remove useless orig_rtr_lsa()
On Tue, Dec 31, 2019 at 01:47:08PM +0100, Denis Fondras wrote:
> Rename orig_rtr_lsa_area() to orig_rtr_lsa()
>
> Now that area is part of iface, original orig_rtr_lsa() is useless. Also
> verifying that area != NULL is not needed in some cases (these are leftovers
> of
> the previous diff).
>
OK remi@
>
> Index: interface.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/interface.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 interface.c
> --- interface.c 23 Dec 2019 07:33:49 - 1.27
> +++ interface.c 31 Dec 2019 12:44:15 -
> @@ -144,7 +144,7 @@ if_fsm(struct iface *iface, enum iface_e
>
> if (iface->state != old_state) {
> area_track(iface->area);
> - orig_rtr_lsa(iface);
> + orig_rtr_lsa(iface->area);
> orig_link_lsa(iface);
>
> /* state change inform RDE */
> @@ -395,7 +395,7 @@ if_act_start(struct iface *iface)
>
> if (iface->cflags & F_IFACE_PASSIVE) {
> /* for an update of stub network entries */
> - orig_rtr_lsa(iface);
> + orig_rtr_lsa(iface->area);
> return (0);
> }
>
> @@ -569,7 +569,7 @@ start:
> nbr_fsm(nbr, NBR_EVT_ADJ_OK);
> }
>
> - orig_rtr_lsa(iface);
> + orig_rtr_lsa(iface->area);
> if (iface->state & IF_STA_DR || old_state & IF_STA_DR)
> orig_net_lsa(iface);
> }
> @@ -586,7 +586,7 @@ if_act_reset(struct iface *iface)
>
> if (iface->cflags & F_IFACE_PASSIVE) {
> /* for an update of stub network entries */
> - orig_rtr_lsa(iface);
> + orig_rtr_lsa(iface->area);
> return (0);
> }
>
> Index: neighbor.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/neighbor.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 neighbor.c
> --- neighbor.c23 Dec 2019 07:33:49 - 1.15
> +++ neighbor.c31 Dec 2019 12:44:15 -
> @@ -202,7 +202,7 @@ nbr_fsm(struct nbr *nbr, enum nbr_event
>* neighbor changed from/to FULL
>* originate new rtr and net LSA
>*/
> - orig_rtr_lsa(nbr->iface);
> + orig_rtr_lsa(nbr->iface->area);
> if (nbr->iface->state & IF_STA_DR)
> orig_net_lsa(nbr->iface);
>
> @@ -226,7 +226,7 @@ nbr_fsm(struct nbr *nbr, enum nbr_event
> nbr_state_name(nbr->state));
>
> if (nbr->iface->type == IF_TYPE_VIRTUALLINK) {
> - orig_rtr_lsa(nbr->iface);
> + orig_rtr_lsa(nbr->iface->area);
> }
> }
>
> Index: ospf6d.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 ospf6d.c
> --- ospf6d.c 16 Dec 2019 08:28:33 - 1.45
> +++ ospf6d.c 31 Dec 2019 12:44:15 -
> @@ -741,7 +741,7 @@ merge_config(struct ospfd_conf *conf, st
> }
> if (a->dirty) {
> a->dirty = 0;
> - orig_rtr_lsa(LIST_FIRST(>iface_list));
> + orig_rtr_lsa(LIST_FIRST(>iface_list)->area);
> }
> }
> }
> Index: ospfe.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.c,v
> retrieving revision 1.59
> diff -u -p -r1.59 ospfe.c
> --- ospfe.c 28 Dec 2019 09:25:24 - 1.59
> +++ ospfe.c 31 Dec 2019 12:44:15 -
> @@ -45,7 +45,6 @@
> void ospfe_sig_handler(int, short, void *);
> __dead void ospfe_shutdown(void);
> void orig_rtr_lsa_all(struct area *);
> -void orig_rtr_lsa_area(struct area *);
> struct iface *find_vlink(struct abr_rtr *);
>
> struct ospfd_conf*oeconf = NULL, *nconf;
> @@ -301,7 +300,7 @@ ospfe_dispatch_main(int fd, short event,
> i->depend_ok =
> ifstate_is_up(ifp);
> if (ifstate_is_up(i))
> - orig_rtr_lsa(i);
> + orig_rtr_lsa(i->area);
> }
> }
> }
> @@ -600,8 +599,6 @@ ospfe_dispatch_rde(int fd, short event,
>* flood on all area interfaces on
>* area 0.0.0.0 include also virtual links.
>*/
> - if (nbr->iface->area == NULL)
> -
PATCH: fix race condition in binutils-2.17 build
We don't normally hit this thanks to the "expensive job" heuristics
(meaning that we don't start the second target while the first is
building), but still there is a race, and if for whatever reason we
remove that heuristics we hit it fairly hard.
The patch is fairly trivial, just run both targets separately.
Example of a crash and analysis follows.
Note that the problem only occurs in binutils-2.17. The former versions
did have one less level of indirection in their Makefiles.
Index: Makefile.bsd-wrapper
===
RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile.bsd-wrapper
--- Makefile.bsd-wrapper21 Dec 2019 21:40:00 - 1.19
+++ Makefile.bsd-wrapper2 Jan 2020 10:13:01 -
@@ -64,6 +64,7 @@ CONFIGTARGET+=--without-gnu-as
.endif
all: config.status
+.for t in all info
SUBDIRS='${SUBDIRS}' \
CONFIGURE_HOST_MODULES='${CONFIGURE_HOST_MODULES}' \
${MAKE} ${XCFLAGS} \
@@ -74,7 +75,8 @@ all: config.status
BSDSRCDIR=${BSDSRCDIR} \
ALL_MODULES="${ALL_MODULES}" \
ALL_HOST_MODULES='${ALL_HOST_MODULES}' \
- INFO_HOST_MODULES='${INFO_HOST_MODULES}' all info
+ INFO_HOST_MODULES='${INFO_HOST_MODULES}' $t
+.endfor
.include
@@ -115,6 +117,7 @@ config.status: do-config
# Need to pass SUBDIRS because of install-info
install: maninstall
+.for t in install install-info
${INSTALL} -d -o ${BINOWN} -g ${BINGRP} \
${DESTDIR}${PREFIX}/libdata/ldscripts
SUBDIRS='${INST_SUBDIRS}' ${MAKE} DESTDIR='${DESTDIR}' \
@@ -126,7 +129,8 @@ install: maninstall
INSTALL_PROGRAM='${INSTALL} -c ${INSTALL_STRIP} -o ${BINOWN} -g
${BINGRP} -m ${BINMODE}' \
INSTALL_DATA='${INSTALL} -c -o ${DOCOWN} -g ${DOCGRP} -m
${NONBINMODE}' \
INSTALL_INFO_HOST_MODULES='${INSTALL_INFO_HOST_MODULES}' \
- install install-info
+ $t
+.endfor
.if ${LINKER_VERSION:L} == "bfd"
rm -f ${DESTDIR}${PREFIX}/bin/ld
ln ${DESTDIR}${PREFIX}/bin/ld.bfd ${DESTDIR}${PREFIX}/bin/ld
Tail of a crashing build:
checking for LC_MESSAGES... (cached) yes
checking for LC_MESSAGES... (cached) yes
checking whether NLS is requested... no
checking whether NLS is requested... no
updating cache ./config.cache
updating cache ./config.cache
creating ./config.status
creating ./config.status
mv: rename conftest.tail to conftest.vals: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
mv: conftest.tail: No such file or directory
creating Makefile
creating Makefile
./config.status[812]: syntax error: `fi' unexpected
./config.status[812]: syntax error: `fi' unexpected
*** Error 1 in gnu/usr.bin/binutils-2.17/obj (Makefile:5406 'configure-intl':
@r=`${PWDCMD-pwd}`; export r; s=`cd /usr/src/gnu/usr.bin/binu...)
*** Error 1 in gnu/usr.bin/binutils-2.17/obj (Makefile:5406 'configure-intl':
@r=`${PWDCMD-pwd}`; export r; s=`cd /usr/src/gnu/usr.bin/binu...)
*** Error 2 in target 'all'
*** Error 2 in gnu/usr.bin/binutils-2.17/obj (Makefile:747 'do-info':
@r=`${PWDCMD-pwd}`; export r; s=`cd /usr/src/gnu/usr.bin/binutils-2.1...)
*** Error 2 (Makefile:634 'all': @r=`${PWDCMD-pwd}`; export r; s=`cd
/usr/src/gnu/usr.bin/binutils-2.17; ${PWDCMD-pwd}`; export s; if [ -f...)
*** Error 2 in gnu/usr.bin/binutils-2.17 (Makefile.bsd-wrapper:77 'all')
You'll notice the duplicate lines. It's actually libintl running configure
twice at the same time. Initially, I thought "oh let's configure libintl
up-front". Unfortunately this is not enough, and leads to a different crash.
Now, for the generated Makefile. In binutils prior to 2.17, both all and
info go straight info building, so they're not affected.
binutils-2.17 has the following snippets:
all:
@: $(MAKE); $(unstage)
@r=`${PWD_COMMAND}`; export r; \
s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
if [ -f stage_last ]; then \
$(MAKE) $(TARGET_FLAGS_TO_PASS) all-host all-target; \
else \
$(MAKE) $(RECURSE_FLAGS_TO_PASS) all-host all-target; \
fi
(the : $(MAKE) is the fun way automake/autoconf disables maintenance lines)
and
info: do-info
do-info:
@: $(MAKE); $(unstage)
@r=`${PWD_COMMAND}`; export r; \
s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
$(MAKE) $(RECURSE_FLAGS_TO_PASS) info-host \
info-target
so, if they're run at the same time, they will *both* recurse and run
a separate make...
looking a bit further,
