two jot tweaks
If jot is run without arguments, prec will never be changed from -1.
This results in the nonsensical format string "%.-1f" being produced in
getformat(). __vfprintf() will misinterpret the - as a left adjustment
flag and the precision used will be 0. The result is the samw as "%.0f",
which is what we want.
The second tweak is to use de Morgan's rule to get rid of an unnecessary
if scope that sets use_unif to its initialized value.
Index: jot.c
===
RCS file: /cvs/src/usr.bin/jot/jot.c,v
retrieving revision 1.51
diff -u -p -r1.51 jot.c
--- jot.c 30 Jul 2021 02:47:37 - 1.51
+++ jot.c 12 Aug 2021 22:03:47 -
@@ -160,10 +160,10 @@ main(int argc, char *argv[])
mask |= REPS;
if (reps == 0)
infinity = true;
- if (prec == -1)
- prec = 0;
}
case 0:
+ if (prec == -1)
+ prec = 0;
break;
default:
errx(1, "Too many arguments. What do you mean by %s?",
@@ -258,9 +258,7 @@ main(int argc, char *argv[])
}
x = ender - begin;
- if (prec == 0 && (fmod(ender, 1) != 0 || fmod(begin, 1) != 0))
- use_unif = 0;
- else {
+ if (prec > 0 || (fmod(ender, 1) == 0 && fmod(begin, 1) == 0)) {
double range;
while (prec-- > 0)
hw.setperf diff to test on i386
It would be nice if someone running i386 could apply this diff, run
without apmd and check that setting sysctl hw.setperf to different
values between 0 and 100 works and changes hw.cpuspeed as expected.
hw.cpuspeed should take on the all the values reported in dmesg, e.g.,
cpu0: Enhanced SpeedStep 1599 MHz: speeds: 1600, 1400, 1200, 1000, 800, 600 MHz
Then run apmd -A and check that running md5 -ttt makes hw.cpuspeed go
to the maximum and back down to the minimum once md5 is done.
Thanks
Index: sys/arch/i386/i386/est.c
===
RCS file: /cvs/src/sys/arch/i386/i386/est.c,v
retrieving revision 1.53
diff -u -p -r1.53 est.c
--- sys/arch/i386/i386/est.c12 Aug 2021 15:16:23 - 1.53
+++ sys/arch/i386/i386/est.c12 Aug 2021 15:42:17 -
@@ -78,6 +78,7 @@
struct est_op {
uint16_t ctrl;
uint16_t mhz;
+ uint16_t pct;
};
/* Ultra Low Voltage Intel Pentium M processor 900 MHz */
@@ -973,10 +974,16 @@ est_acpi_init(void)
struct acpicpu_pss *pss;
struct fqlist *acpilist;
int nstates, i;
+ int high, low;
if ((nstates = acpicpu_fetch_pss()) == 0)
goto nolist;
+ high = pss[0].pss_core_freq;
+ low = pss[nstates - 1].pss_core_freq;
+ if (high - low <= 0)
+ goto nolist;
+
if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
== NULL)
goto nolist;
@@ -990,6 +997,8 @@ est_acpi_init(void)
for (i = 0; i < nstates; i++) {
acpilist->table[i].mhz = pss[i].pss_core_freq;
acpilist->table[i].ctrl = pss[i].pss_ctrl;
+ acpilist->table[i].pct =
+ (pss[i].pss_core_freq - low) * 100 / (high - low);
}
acpicpu_set_notify(est_acpi_pss_changed);
@@ -1008,12 +1017,21 @@ est_acpi_pss_changed(struct acpicpu_pss
{
struct fqlist *acpilist;
int needtran = 1, i;
+ int high, low;
u_int64_t msr;
u_int16_t cur;
msr = rdmsr(MSR_PERF_STATUS);
cur = msr & 0x;
+ high = pss[0].pss_core_freq;
+ low = pss[npss - 1].pss_core_freq;
+ if (high - low <= 0) {
+ printf("est_acpi_pss_changed: new est state has no "
+ "speed step\n");
+ return;
+ }
+
if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
== NULL) {
printf("est_acpi_pss_changed: cannot allocate memory for new "
@@ -1032,6 +1050,8 @@ est_acpi_pss_changed(struct acpicpu_pss
for (i = 0; i < npss; i++) {
acpilist->table[i].mhz = pss[i].pss_core_freq;
acpilist->table[i].ctrl = pss[i].pss_ctrl;
+ acpilist->table[i].pct =
+ (pss[i].pss_core_freq - low) * 100 / (high - low);
if (pss[i].pss_ctrl == cur)
needtran = 0;
}
@@ -1152,18 +1172,25 @@ est_init(struct cpu_info *ci, int vendor
printf("%s: using only highest and lowest power "
"states\n", cpu_device);
+ fake_table[0].pct = 51;
+
fake_table[1].ctrl = idlo;
fake_table[1].mhz = MSR2MHZ(idlo, bus_clock);
+ fake_table[1].pct = 0;
fake_fqlist->n = 2;
} else {
printf("%s: using only highest, current and lowest "
"power states\n", cpu_device);
+ fake_table[0].pct = 67;
+
fake_table[1].ctrl = cur;
fake_table[1].mhz = MSR2MHZ(cur, bus_clock);
+ fake_table[1].pct = 34;
fake_table[2].ctrl = idlo;
fake_table[2].mhz = MSR2MHZ(idlo, bus_clock);
+ fake_table[2].pct = 0;
fake_fqlist->n = 3;
}
@@ -1218,10 +1245,10 @@ est_setperf(int level)
if (est_fqlist == NULL)
return;
- i = ((level * est_fqlist->n) + 1) / 101;
- if (i >= est_fqlist->n)
- i = est_fqlist->n - 1;
- i = est_fqlist->n - 1 - i;
+ for (i = 0; i < est_fqlist->n; i++) {
+ if (level >= est_fqlist->table[i].pct)
+ break;
+ }
msr = rdmsr(MSR_PERF_CTL);
msr &= ~0xULL;
ure: Remove unused ure_stop_task
Hi,
The ure_stop_task is not scheduled anywhere. In fact, it has never been
used. Remove it.
So long,
- Christian
---
sys/dev/usb/if_ure.c| 3 ---
sys/dev/usb/if_urereg.h | 1 -
2 files changed, 4 deletions(-)
diff --git a/sys/dev/usb/if_ure.c b/sys/dev/usb/if_ure.c
index ea73db00954..03b190821c4 100644
--- a/sys/dev/usb/if_ure.c
+++ b/sys/dev/usb/if_ure.c
@@ -1626,8 +1626,6 @@ ure_attach(struct device *parent, struct device *self,
void *aux)
usb_init_task(>ure_tick_task, ure_tick_task, sc,
USB_TASK_TYPE_GENERIC);
- usb_init_task(>ure_stop_task, (void (*)(void *))ure_stop, sc,
- USB_TASK_TYPE_GENERIC);
id = usbd_get_interface_descriptor(sc->ure_iface);
@@ -1795,7 +1793,6 @@ ure_detach(struct device *self, int flags)
usbd_abort_pipe(sc->ure_ep[URE_ENDPT_RX]);
usb_rem_task(sc->ure_udev, >ure_tick_task);
- usb_rem_task(sc->ure_udev, >ure_stop_task);
s = splusb();
diff --git a/sys/dev/usb/if_urereg.h b/sys/dev/usb/if_urereg.h
index b000b4b4dc8..8f02f0bc396 100644
--- a/sys/dev/usb/if_urereg.h
+++ b/sys/dev/usb/if_urereg.h
@@ -527,7 +527,6 @@ struct ure_softc {
/* usb */
struct usbd_interface *ure_iface;
struct usb_task ure_tick_task;
- struct usb_task ure_stop_task;
int ure_ed[URE_ENDPT_MAX];
struct usbd_pipe*ure_ep[URE_ENDPT_MAX];
--
2.32.0
Re: fresh prompt after Ctrl-C in cdio(1)
Ingo Schwarze: > deraadt@ recently pointed out to me in private mail that it is good > for usability if interactive programs providing line editing > functionality are consistent what they do with Ctrl-C, ideally > discard the current input line and provide a fresh prompt. > > So i propose to do the same in cdio(1), which currently just exits > on Ctrl-C. > > OK? That looks correct and works fine for me. ok naddy@ (I still have a USB CD drive and I use it from time to time to play audio CDs with cdio. I had completely forgotten that cdio supports editline, though. The cdio interactive mode is pretty useless since interrupting a running command aborts the program.) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: fresh prompt after Ctrl-C in cdio(1)
Hi Martijn, Martijn van Duren wrote on Thu, Aug 12, 2021 at 04:37:24PM +0200: > Maybe I've used cdio once or twice and I don't have a cd-player at hand > (at least connected to my workstation) to test this. So purely from code > inspection: You set the signal handler before entering el_gets and you > ignore it right after. More precisely, i restore default signal handling right after, which is "terminate process" for SIGINT. In most situations, i dislike ignoring SIGINT. > Wouldn't this imply that if you do something like "cdplay" at the prompt > and while you wait for the cd to spin you hit ^C the application just > exits? Yes. > If so, wouldn't it make more sense to install the signal handler once > and let open_cd() handle EINTR as well and just return to the prompt? Maybe. The question whether cdio(1) should allow Ctrl-C to be used to interrupt more operations, and not just command line editing, and return to the interactive prompt in that case is certainly legitimate. Then again, my point is to unify line editing across different programs, not to consider overall signal handling in cdio(1). As long as we only touch line editing in cdio(1), that goal can be reached and the risk of regressions is easier to control. Extending signal handling over larger swaths of code requires reviewing more code and making sure what Ctrl-C does is sane in all code paths in all of that code. For example, looking at the function run(), i see that most commands start by calling open_cd(), but not all do. A few do not call it at all (e.g. CMD_SET, CMD_HELP), in which case we would have to make sure that the signal handler does not remain installed longer than intended, a few do some other work before calling it (e.g. CMD_DEVICE). So i would prefer to leave the broader question of signal handling in cdio(1), outside the specific domain of command line editing, to people who are more familiar with the code and usage of cdio(1). If somebody wants to develop, test and propose a patch with a wider scope, for example a global signal handler that can interupt any operation - such a patch would have to make sure cleanup is done as required after all operations that might get aborted -, i'm not opposed to that, but it don't feel like developing such a patch myself at this point. Yours, Ingo
Re: fresh prompt after Ctrl-C in cdio(1)
On Thu, 2021-08-12 at 15:57 +0200, Ingo Schwarze wrote:
> Hi,
>
> deraadt@ recently pointed out to me in private mail that it is good
> for usability if interactive programs providing line editing
> functionality are consistent what they do with Ctrl-C, ideally
> discard the current input line and provide a fresh prompt.
> At least that is what bc(1), ftp(1), sftp(1) and all shells do.
>
> So i propose to do the same in cdio(1), which currently just exits
> on Ctrl-C.
>
> Sending to tech@ because i'm not quite sure which developer owns cdio(1),
> or who might be interested. According to the CVS logs, naddy@ was the
> last one to change user-visible functionality, and before that, there
> was nothing but bug fixes and cleanup since 2010.
>
> OK?
> Ingo
>
> P.S.
> Note that the current "!siz" is fragile; el_gets(3) sets it to -1
> on error. I'm polishing that while here.
>
> P.P.S.
> buf = (char *)el_gets(...) is ugly and potentially dangerous, the
> manual page explicitly warns to not do that, but that's a job for
> another day.
>
> P.P.P.S.
> Insects are the most successful class of animals on earth.
> Even in OpenBSD, it is rarely possible to look at code without
> finding something unrelated that could be improved, too.
Maybe I've used cdio once or twice and I don't have a cd-player at hand
(at least connected to my workstation) to test this. So purely from code
inspection: You set the signal handler before entering el_gets and you
ignore it right after.
Wouldn't this imply that if you do something like "cdplay" at the prompt
and while you wait for the cd to spin you hit ^C the application just
exits? If so, wouldn't it make more sense to install the signal handler
once and let open_cd() handle EINTR as well and just return to the prompt?
Something that springs to is
cdio> play
^C
cdio> cdplay
>
>
> Index: cdio.c
> ===
> RCS file: /cvs/src/usr.bin/cdio/cdio.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 cdio.c
> --- cdio.c 18 Jan 2021 00:44:00 - 1.80
> +++ cdio.c 12 Aug 2021 13:36:38 -
> @@ -63,6 +63,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -158,6 +159,7 @@ int verbose = 1;
> intmsf = 1;
> const char *cddb_host;
> char **track_names;
> +volatile sig_atomic_t signo;
>
> EditLine *el = NULL; /* line-editing structure */
> History*hist = NULL; /* line-editing history */
> @@ -179,6 +181,7 @@ int pstatus(char *arg);
> intplay_next(char *arg);
> intplay_prev(char *arg);
> intplay_same(char *arg);
> +void sigint_handler(int);
> char *input(int *);
> char *prompt(void);
> void prtrack(struct cd_toc_entry *e, int lastflag, char *name);
> @@ -1499,18 +1502,36 @@ status(int *trk, int *min, int *sec, int
> return s.data->header.audio_status;
> }
>
> +void
> +sigint_handler(int signo_arg)
> +{
> + signo = signo_arg;
> +}
> +
> char *
> input(int *cmd)
> {
> + struct sigaction sa;
> char *buf;
> int siz = 0;
> char *p;
> HistEvent hev;
>
> + memset(, 0, sizeof(sa));
> do {
> - if ((buf = (char *) el_gets(el, )) == NULL || !siz) {
> - *cmd = CMD_QUIT;
> + signo = 0;
> + sa.sa_handler = sigint_handler;
> + if (sigaction(SIGINT, , NULL) == -1)
> + err(1, "sigaction");
> + buf = (char *)el_gets(el, );
> + sa.sa_handler = SIG_DFL;
> + if (sigaction(SIGINT, , NULL) == -1)
> + err(1, "sigaction");
> + if (buf == NULL || siz <= 0) {
> fprintf(stderr, "\r\n");
> + if (siz < 0 && errno == EINTR && signo == SIGINT)
> + continue;
> + *cmd = CMD_QUIT;
> return (0);
> }
> if (strlen(buf) > 1)
>
fresh prompt after Ctrl-C in cdio(1)
Hi,
deraadt@ recently pointed out to me in private mail that it is good
for usability if interactive programs providing line editing
functionality are consistent what they do with Ctrl-C, ideally
discard the current input line and provide a fresh prompt.
At least that is what bc(1), ftp(1), sftp(1) and all shells do.
So i propose to do the same in cdio(1), which currently just exits
on Ctrl-C.
Sending to tech@ because i'm not quite sure which developer owns cdio(1),
or who might be interested. According to the CVS logs, naddy@ was the
last one to change user-visible functionality, and before that, there
was nothing but bug fixes and cleanup since 2010.
OK?
Ingo
P.S.
Note that the current "!siz" is fragile; el_gets(3) sets it to -1
on error. I'm polishing that while here.
P.P.S.
buf = (char *)el_gets(...) is ugly and potentially dangerous, the
manual page explicitly warns to not do that, but that's a job for
another day.
P.P.P.S.
Insects are the most successful class of animals on earth.
Even in OpenBSD, it is rarely possible to look at code without
finding something unrelated that could be improved, too.
Index: cdio.c
===
RCS file: /cvs/src/usr.bin/cdio/cdio.c,v
retrieving revision 1.80
diff -u -p -r1.80 cdio.c
--- cdio.c 18 Jan 2021 00:44:00 - 1.80
+++ cdio.c 12 Aug 2021 13:36:38 -
@@ -63,6 +63,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -158,6 +159,7 @@ int verbose = 1;
intmsf = 1;
const char *cddb_host;
char **track_names;
+volatile sig_atomic_t signo;
EditLine *el = NULL; /* line-editing structure */
History*hist = NULL; /* line-editing history */
@@ -179,6 +181,7 @@ int pstatus(char *arg);
intplay_next(char *arg);
intplay_prev(char *arg);
intplay_same(char *arg);
+void sigint_handler(int);
char *input(int *);
char *prompt(void);
void prtrack(struct cd_toc_entry *e, int lastflag, char *name);
@@ -1499,18 +1502,36 @@ status(int *trk, int *min, int *sec, int
return s.data->header.audio_status;
}
+void
+sigint_handler(int signo_arg)
+{
+ signo = signo_arg;
+}
+
char *
input(int *cmd)
{
+ struct sigaction sa;
char *buf;
int siz = 0;
char *p;
HistEvent hev;
+ memset(, 0, sizeof(sa));
do {
- if ((buf = (char *) el_gets(el, )) == NULL || !siz) {
- *cmd = CMD_QUIT;
+ signo = 0;
+ sa.sa_handler = sigint_handler;
+ if (sigaction(SIGINT, , NULL) == -1)
+ err(1, "sigaction");
+ buf = (char *)el_gets(el, );
+ sa.sa_handler = SIG_DFL;
+ if (sigaction(SIGINT, , NULL) == -1)
+ err(1, "sigaction");
+ if (buf == NULL || siz <= 0) {
fprintf(stderr, "\r\n");
+ if (siz < 0 && errno == EINTR && signo == SIGINT)
+ continue;
+ *cmd = CMD_QUIT;
return (0);
}
if (strlen(buf) > 1)
Re: missing newlines in est.c printfs
On Thu, Aug 12, 2021 at 07:47:36AM +0200, Theo Buehler wrote:
> On Thu, Aug 12, 2021 at 03:01:37PM +1000, Jonathan Gray wrote:
> > On Thu, Aug 12, 2021 at 06:44:51AM +0200, Theo Buehler wrote:
> > > There doesn't seem to be a good reason for omitting the newlines here.
> > > If those are ever hit, it will look odd. Am I missing something?
> >
> > See the i386 p3_get_bus_clock() which is where this came from.
> > i386 prints the msr value and a newline.
> >
> > Wether that part is added to amd64 or removed from i386 it would be best
> > to keep them in sync as much as possible.
>
> Thanks. This syncs the print_msr code path from i386 in p3_get_bus_clock
> and adds the missing newlines in est_acpi_pss_changed() in i386.
ok jsg@
>
> Index: sys/arch/i386/i386/est.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/est.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 est.c
> --- sys/arch/i386/i386/est.c 31 Mar 2018 13:45:03 - 1.52
> +++ sys/arch/i386/i386/est.c 12 Aug 2021 05:46:12 -
> @@ -1017,14 +1017,14 @@ est_acpi_pss_changed(struct acpicpu_pss
> if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
> == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "est state");
> + "est state\n");
> return;
> }
>
> if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
> M_DEVBUF, M_NOWAIT)) == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "operating points");
> + "operating points\n");
> free(acpilist, M_DEVBUF, sizeof(*acpilist));
> return;
> }
> Index: sys/arch/amd64/amd64/est.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/est.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 est.c
> --- sys/arch/amd64/amd64/est.c11 Aug 2021 18:15:50 - 1.40
> +++ sys/arch/amd64/amd64/est.c12 Aug 2021 05:32:49 -
> @@ -187,7 +187,7 @@ p3_get_bus_clock(struct cpu_info *ci)
> default:
> printf("%s: unknown Core FSB_FREQ value %d",
> ci->ci_dev->dv_xname, bus);
> - break;
> + goto print_msr;
> }
> break;
> case 0x1c: /* Atom */
> @@ -211,13 +211,20 @@ p3_get_bus_clock(struct cpu_info *ci)
> default:
> printf("%s: unknown Atom FSB_FREQ value %d",
> ci->ci_dev->dv_xname, bus);
> - break;
> + goto print_msr;
> }
> break;
> default:
> /* no FSB on modern Intel processors */
> break;
> }
> + return;
> +print_msr:
> + /*
> + * Show the EBL_CR_POWERON MSR, so we'll at least have
> + * some extra information, such as clock ratio, etc.
> + */
> + printf(" (0x%llx)\n", rdmsr(MSR_EBL_CR_POWERON));
> }
>
> #if NACPICPU > 0
> @@ -291,14 +298,14 @@ est_acpi_pss_changed(struct acpicpu_pss
> if ((acpilist = malloc(sizeof(struct fqlist), M_DEVBUF, M_NOWAIT))
> == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "est state");
> + "est state\n");
> return;
> }
>
> if ((acpilist->table = mallocarray(npss, sizeof(struct est_op),
> M_DEVBUF, M_NOWAIT)) == NULL) {
> printf("est_acpi_pss_changed: cannot allocate memory for new "
> - "operating points");
> + "operating points\n");
> free(acpilist, M_DEVBUF, sizeof(struct fqlist));
> return;
> }
>
>
