Re: CVS commit: src/bin/sleep
Date:Thu, 24 Jan 2019 16:18:49 +0100 From:Joerg Sonnenberger Message-ID: <20190124151849.ga10...@britannica.bec.de> | This is overcomplicated and fragile, IMO. ps: if the fragility referred to is that it might now switch mid-stream into sending messages in English rather than in the locale's language - then that is a valid concern, and I could certainly change it to use strtod_l() in that case to avoid that problem. Of course, that only becomes an issue when someone writes a messages catalog for sleep's usage message, and the SIGINFO messages, and provides all of the locale specific translations, and then fixes the code to cause them to be used. The only other message that sleep ever produces is from the err(EXIT_FAILURE, "nanosleep failed"); which should produce the strerror() result in a form appropriate for the local locale -- but that one only happens when sleep is given a very large argument, which would be better handled by simply not calling nanosleep, but using sleep(3) instead - anyone who believes that sleep 2000.25 is a useful thing to do is delusional (if we fall back to sleep the fractional seconds part would simply be ignored, not an error). Sleep doesn't have an error return, and nanosleep() would no longer ever fail, so the err() (the one place that a locale specific message is produced now) would no longer be needed in practice (it would still be in the code, just in case, but would never fire, so what language it might produce its message in would not matter.) For sleep durations outside the range of what sleep(3) permits, we should just pause(), no-one will ever know the difference. kre
Re: CVS commit: src/sys/dev/pci
On 2019/01/25 3:08, Christos Zoulas wrote:
In article <20190124045004.c9f48f...@cvs.netbsd.org>,
SAITOH Masanobu wrote:
-=-=-=-=-=-
Module Name:src
Committed By: msaitoh
Date: Thu Jan 24 04:50:04 UTC 2019
Modified Files:
src/sys/dev/pci: if_wm.c
Log Message:
No functional change intended:
- Use "do {} while (/*CONSTCOND*/false)" for null DPRINTF().
- Reduce indent level of wm_linkintr_gmii().
There is __nothing
christos
I didn't know it! I've changed with it now.
Thanks.
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
Re: CVS commit: src/bin/sleep
Date:Thu, 24 Jan 2019 16:18:49 +0100
From:Joerg Sonnenberger
Message-ID: <20190124151849.ga10...@britannica.bec.de>
| This is overcomplicated and fragile, IMO. Can we just go back to the old
| code and switch the strtod to strtod_l with LC_C_LOCALE? That solves the
| input problem.
We could certainly do that, but while it is a little complicated, I do not
really think it is fragile. Using a locale specific decimal radix in a
script is fragile - but the only way to avoid that is either to effectively
give up on non-C locales entirely for scripts, or drop all support for
fractional numbers as args to any command.
Note: the arg to "sleep" might come as ...
echo -n "How many seconds do you want to sleep? "
read secs
sleep "${secs}"
(with some error checking).
I have no idea why sleep() was made to parse its arg in a locale
specific way, but that was done long long ago, and was (according
to the comments) done quite deliberately.
I think changing that decison (rather than just avoiding the problem,
as the current "fix" does) needs more discussion than a few comments
on the source-changes-d list.
kre
ps: "long long ago" means 1997 - this was added in version 1.10 of
sleep.c along with the comment explaining that it allows a locale
specific radix to be used. 1997 means this was in NetBSD 1.3 and
has been in all versions since. Even if we were to change that for
NetBSD 9, I don't think such an alteration should be done in a point
release, so we will need to make something deal with the problem
for 8.1
Re: CVS commit: src/sys/dev/usb
On Thu, Jan 24, 2019 at 03:51:02PM +0100, Robert Swindells wrote: > "Michael van Elst" wrote: > > Module Name:src > > Committed By: mlelstv > > Date: Sat Jan 5 07:56:07 UTC 2019 > > > > Modified Files: > >src/sys/dev/usb: if_mue.c if_muevar.h > > > > Log Message: > > Enable multiple outstanding transfers. > > > > iperf3 now shows 250MBit/s for sending and 225MBit/s for receiving. > > Which device was this tested on ? > > It doesn't work at all for me on a LAN7500. Tested on an RPI3b+ which is LAN7800. But I now see some inconsistent performance for receiving. Example: client is netbsd-8/i386 re0, server is RPI3b+ % iperf3 -c jowlson [ 6] local 10.28.5.2 port 54509 connected to 10.28.5.23 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 6] 0.00-1.00 sec 12.8 MBytes 107 Mbits/sec0 55.1 KBytes [ 6] 1.00-2.00 sec 23.7 MBytes 199 Mbits/sec0 28.3 KBytes [ 6] 2.00-3.00 sec 25.7 MBytes 216 Mbits/sec0 41.0 KBytes [ 6] 3.00-4.00 sec 25.6 MBytes 215 Mbits/sec0 28.3 KBytes [ 6] 4.00-5.00 sec 25.7 MBytes 215 Mbits/sec0 52.3 KBytes [ 6] 5.00-6.00 sec 25.6 MBytes 214 Mbits/sec0 46.7 KBytes [ 6] 6.00-7.00 sec 25.6 MBytes 215 Mbits/sec0 53.7 KBytes [ 6] 7.00-8.00 sec 25.7 MBytes 215 Mbits/sec0 31.1 KBytes [ 6] 8.00-9.00 sec 25.6 MBytes 215 Mbits/sec0 31.1 KBytes [ 6] 9.00-10.00 sec 25.7 MBytes 215 Mbits/sec0 38.2 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 6] 0.00-10.00 sec 242 MBytes 203 Mbits/sec0 sender [ 6] 0.00-10.00 sec 241 MBytes 202 Mbits/sec receiver Example2: client is netbsd-7/amd64 wm0 Connecting to host jowlson, port 5201 [ 6] local 10.28.5.19 port 64879 connected to 10.28.5.23 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 6] 0.00-1.00 sec 14.3 MBytes 120 Mbits/sec0 2.83 KBytes [ 6] 1.00-2.01 sec 12.1 MBytes 99.9 Mbits/sec0 2.83 KBytes [ 6] 2.01-3.00 sec 4.82 MBytes 41.0 Mbits/sec1 41.0 KBytes [ 6] 3.00-4.00 sec 17.5 MBytes 147 Mbits/sec0 29.7 KBytes [ 6] 4.00-5.00 sec 2.23 MBytes 18.6 Mbits/sec0 2.83 KBytes [ 6] 5.00-6.01 sec 8.12 MBytes 68.0 Mbits/sec1 2.83 KBytes [ 6] 6.01-7.00 sec 2.89 MBytes 24.4 Mbits/sec1 24.0 KBytes [ 6] 7.00-8.01 sec 6.67 MBytes 55.5 Mbits/sec0 1.41 KBytes [ 6] 8.01-9.00 sec 7.06 MBytes 59.7 Mbits/sec4 4.24 KBytes [ 6] 9.00-10.01 sec 6.43 MBytes 53.4 Mbits/sec0 2.83 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 6] 0.00-10.01 sec 82.1 MBytes 68.8 Mbits/sec7 sender [ 6] 0.00-10.01 sec 82.0 MBytes 68.7 Mbits/sec receiver notice the Retr column, somewhere packets get lost. The reverse direction (-R) is fine in both setups. Greetings, -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/dev/pci
In article <20190124045004.c9f48f...@cvs.netbsd.org>,
SAITOH Masanobu wrote:
>-=-=-=-=-=-
>
>Module Name: src
>Committed By: msaitoh
>Date: Thu Jan 24 04:50:04 UTC 2019
>
>Modified Files:
> src/sys/dev/pci: if_wm.c
>
>Log Message:
>No functional change intended:
>- Use "do {} while (/*CONSTCOND*/false)" for null DPRINTF().
>- Reduce indent level of wm_linkintr_gmii().
There is __nothing
christos
Re: CVS commit: src/sys/dev/usb
"Michael van Elst" wrote: Module Name:src Committed By: mlelstv Date: Sat Jan 5 07:56:07 UTC 2019 Modified Files: src/sys/dev/usb: if_mue.c if_muevar.h Log Message: Enable multiple outstanding transfers. iperf3 now shows 250MBit/s for sending and 225MBit/s for receiving. Which device was this tested on ? It doesn't work at all for me on a LAN7500. Robert Swindells
Re: CVS commit: src/bin/sleep
On Thu, Jan 24, 2019 at 16:18:49 +0100, Joerg Sonnenberger wrote: > Date: Thu, 24 Jan 2019 16:18:49 +0100 > From: Joerg Sonnenberger > Subject: Re: CVS commit: src/bin/sleep > To: source-changes-d@NetBSD.org > Mail-Followup-To: source-changes-d@NetBSD.org > w > On Sat, Jan 19, 2019 at 01:27:12PM +, Robert Elz wrote: > > Module Name:src > > Committed By: kre > > Date: Sat Jan 19 13:27:12 UTC 2019 > > > > Modified Files: > > src/bin/sleep: sleep.c > > > > Log Message: > > Allow the decimal radix character '.' to work, regardless of > > what the current locale's radix character happens to be, > > while still allowing locale specific entry of fractional > > seconds (ie: if you're in locale where the radix character > > is ',' you san use "sleep 2.5" or "sleep 2,5" and they > > accomplish the same thing). > > This is overcomplicated and fragile, IMO. Can we just go back to the old > code and switch the strtod to strtod_l with LC_C_LOCALE? That solves the > input problem. Seconded. Accepting locale specific number formats here is quite against POLA, imho. -uwe
Re: CVS commit: src/bin/sleep
On Sat, Jan 19, 2019 at 01:27:12PM +, Robert Elz wrote: > Module Name: src > Committed By: kre > Date: Sat Jan 19 13:27:12 UTC 2019 > > Modified Files: > src/bin/sleep: sleep.c > > Log Message: > Allow the decimal radix character '.' to work, regardless of > what the current locale's radix character happens to be, > while still allowing locale specific entry of fractional > seconds (ie: if you're in locale where the radix character > is ',' you san use "sleep 2.5" or "sleep 2,5" and they > accomplish the same thing). This is overcomplicated and fragile, IMO. Can we just go back to the old code and switch the strtod to strtod_l with LC_C_LOCALE? That solves the input problem. Joerg
re: CVS commit: [pgoyette-compat] src/sys/compat/netbsd32
Just for sanity's sake, I will take another look at this change and see if there's another way to handle it. On Thu, 24 Jan 2019, Paul Goyette wrote: On Thu, 24 Jan 2019, matthew green wrote: Module Name:src Committed By: pgoyette Date: Thu Jan 24 04:24:52 UTC 2019 Modified Files: src/sys/compat/netbsd32 [pgoyette-compat]: netbsd32_sysctl.c Log Message: Use the hook to get the value of machine32 hm. why does the module need to use the hook? doesn't it know the value internally, since it has to publish it via the hook? i don't understand the value of using hooks inside the publisher. they are useful for external consumers, i thought. In this case the symbol is defined as __weak and might not exist. So we use a hook function to return a value if the hook has been set, or a default value if the hook has not been set. The hook's struct itself is always present/allocated. I suppose I could have avoided the hook by simply having another machine32_ptr variable, rather than a hook function. But I wasn't totally clear on whether there were any race conditions during a module unload which could result in retrieving a pointer to a value which just got removed from memory. Of course, if we had a reasonable way of dealing with weak symbols, this mess would not be needed. +--+--++ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org | +--+--++ +--+--++ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org | +--+--++
re: CVS commit: [pgoyette-compat] src/sys/compat/netbsd32
On Thu, 24 Jan 2019, matthew green wrote: Module Name:src Committed By: pgoyette Date: Thu Jan 24 04:24:52 UTC 2019 Modified Files: src/sys/compat/netbsd32 [pgoyette-compat]: netbsd32_sysctl.c Log Message: Use the hook to get the value of machine32 hm. why does the module need to use the hook? doesn't it know the value internally, since it has to publish it via the hook? i don't understand the value of using hooks inside the publisher. they are useful for external consumers, i thought. In this case the symbol is defined as __weak and might not exist. So we use a hook function to return a value if the hook has been set, or a default value if the hook has not been set. The hook's struct itself is always present/allocated. I suppose I could have avoided the hook by simply having another machine32_ptr variable, rather than a hook function. But I wasn't totally clear on whether there were any race conditions during a module unload which could result in retrieving a pointer to a value which just got removed from memory. Of course, if we had a reasonable way of dealing with weak symbols, this mess would not be needed. +--+--++ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org | +--+--++
re: CVS commit: [pgoyette-compat] src/sys/compat/netbsd32
> Module Name: src > Committed By: pgoyette > Date: Thu Jan 24 04:24:52 UTC 2019 > > Modified Files: > src/sys/compat/netbsd32 [pgoyette-compat]: netbsd32_sysctl.c > > Log Message: > Use the hook to get the value of machine32 hm. why does the module need to use the hook? doesn't it know the value internally, since it has to publish it via the hook? i don't understand the value of using hooks inside the publisher. they are useful for external consumers, i thought. .mrg.
