Re: Does grep need a version?
> Bob Beck wrote: > > Since I can't think of a sane reason to check this and care in any > > script, yes, kill it. > > ok beck@ > > Theo pointed out that there may be ports that check the version. I was > considering that too. However, we haven't bumped from 0.9 since 2003. > ggrep is now on 2.22, so I have hope that this isn't the case anymore. I was only pointing out something in ports may want -V to do something, rather than error out poorly. That's all. As to what version it spits out, nothing will care. 0.9, 3.14, 999, nothing will care.
Re: ksh(1): utf8 in emacs editing mode
On Tue, Dec 08, 2015 at 01:19:35AM +0100, Ingo Schwarze wrote: > Hi, > > i'd like to propose a simplified version of this patch Frederic Nowak > posted a few weeks ago for commit. Our experience is probably > not yet sufficient to develop a full-blown solution for all UTF-8 > problems in ksh(1), but this is very non-intrusive and makes the > following commands better without breaking anything: > > * Ctrl-b, Ctrl-xd, Esc-[D, Esc-OD (backward-char) > * Ctrl-f, Ctrl-xc, Esc-[C, Esc-OC (forward-char) > * Ctrl-h, Ctrl-? (delete-char-backward) > * Esc-[3~ (delete-char-forward) > * Ctrl-k (kill-to-eol) > Hi Ingo, While testing ksh, I found the following problem, that I will try to describe. But I dunno if it is just a case no-managed by your patch. It looks like a problem in "inserting a UTF-8 char inside the line (opposed to 'at end of line')". Else your patch is pretty readable, and it would be a good first step. # type echo aàa $ echo aàa ^ cursor here # move two char backward (using arrow or using Ctrl-b) $ echo aàa ^ cursor here # add 'é' char (UTF-8) (there are no problem with non UTF-8 char like 'b') # => visual problem starts $ echo àaa ^ cursor here # hit enter $ echo àaa aéàa ### another way (maybe more simple): # type echo abc $ echo abc ^ cursor here # go start-of-line with Ctrl+A $ echo abc ^ cursor here # insert é UTF-char $echo abcc ^ cursor here in the same manner, it is just a "visual effect", as the line is really "éecho abc" (tested with cut/paste: Ctrl+U Ctrl+Y) Thanks -- Sebastien Marie
Re: Does grep need a version?
Bob Beck wrote: > Since I can't think of a sane reason to check this and care in any > script, yes, kill it. > ok beck@ Theo pointed out that there may be ports that check the version. I was considering that too. However, we haven't bumped from 0.9 since 2003. ggrep is now on 2.22, so I have hope that this isn't the case anymore.
Re: Does grep need a version?
Since I can't think of a sane reason to check this and care in any
script, yes, kill it.
ok beck@
On Mon, Dec 7, 2015 at 10:06 PM, Michael McConville wrote:
> It's been 0.9 since the original import in 2003...
>
>
> Index: grep.1
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.1,v
> retrieving revision 1.43
> diff -u -p -r1.43 grep.1
> --- grep.1 13 Jan 2015 04:45:34 - 1.43
> +++ grep.1 8 Dec 2015 04:59:35 -
> @@ -244,9 +244,6 @@ Nonexistent and unreadable files are ign
> (i.e. their error messages are suppressed).
> .It Fl U
> Search binary files, but do not attempt to print them.
> -.It Fl V
> -Display version information.
> -All other options are ignored.
> .It Fl v
> Selected lines are those
> .Em not
> Index: grep.c
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 grep.c
> --- grep.c 28 Nov 2015 01:17:12 - 1.55
> +++ grep.c 8 Dec 2015 04:59:35 -
> @@ -137,7 +137,6 @@ static const struct option long_options[
> {"basic-regexp",no_argument,NULL, 'G'},
> {"with-filename", no_argument,NULL, 'H'},
> {"binary", no_argument,NULL, 'U'},
> - {"version", no_argument,NULL, 'V'},
> {"text",no_argument,NULL, 'a'},
> {"byte-offset", no_argument,NULL, 'b'},
> {"count", no_argument,NULL, 'c'},
> @@ -328,10 +327,6 @@ main(int argc, char *argv[])
> break;
> case 'U':
> binbehave = BIN_FILE_BIN;
> - break;
> - case 'V':
> - fprintf(stderr, "grep version %u.%u\n", VER_MAJ,
> VER_MIN);
> - exit(0);
> break;
> #ifndef NOZ
> case 'Z':
> Index: grep.h
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 grep.h
> --- grep.h 7 Dec 2015 18:50:06 - 1.23
> +++ grep.h 8 Dec 2015 04:59:35 -
> @@ -34,9 +34,6 @@
> #include
> #include
>
> -#define VER_MAJ 0
> -#define VER_MIN 9
> -
> #define BIN_FILE_BIN 0
> #define BIN_FILE_SKIP 1
> #define BIN_FILE_TEXT 2
>
Re: 3rd party Xbox 360 USB controller support
On 12/07 12:52, Martin Pieuchot wrote:
> On 05/12/15(Sat) 15:22, Christian Heckendorf wrote:
> > The previous thread[1] discussing these controllers includes two
> > patches but they seem to have been merged for the commit in a way
> > that limits support to only Microsoft controllers. 3rd party Xbox 360
> > controllers have their own vendor and product IDs but use the same
> > subclass and protocol as the Microsoft controllers.
> >
> > Here's a diff based on the first patch that will match controllers
> > and assign the report descriptor more generally using subclass/protocol
> > rather than vendor/product. Is it more correct to create an array
> > of known vendors/products and match against a call to usb_lookup()?
>
> It's fine since the same check is used in uhidev_use_rdesc().
>
> Could you try this on a Microsoft controller? Does it still work?
> Jeremy could you check this out?
It still attaches when using a Microsoft controller:
uhidev2 at uhub2 port 1 configuration 1 interface 0 "\M-)Microsoft Corporation
Controller" rev 2.00/1.14 addr 3
uhidev2: iclass 255/93
uhid2 at uhidev2: input=20, output=0, feature=0
ugen0 at uhub2 port 1 configuration 1 "\M-)Microsoft Corporation Controller"
rev 2.00/1.14 addr 3
The SDL joystick program I use for testing works fine with the patch.
Thanks,
Jeremy
>
> >
> > [1] http://marc.info/?l=openbsd-tech&m=138229619410284&w=2
> >
> > Thanks,
> > Christian
> >
> >
> > Index: uhidev.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> > retrieving revision 1.70
> > diff -u -p -r1.70 uhidev.c
> > --- uhidev.c28 Feb 2015 08:42:41 - 1.70
> > +++ uhidev.c5 Dec 2015 20:14:49 -
> > @@ -62,7 +62,10 @@
> > #ifndef SMALL_KERNEL
> > /* Replacement report descriptors for devices shipped with broken ones */
> > #include
> > -int uhidev_use_rdesc(struct uhidev_softc *, int, int, void **, int *);
> > +int uhidev_use_rdesc(struct uhidev_softc *, usb_interface_descriptor_t *,
> > + int, int, void **, int *);
> > +#define UISUBCLASS_XBOX360_CONTROLLER 0x5d
> > +#define UIPROTO_XBOX360_GAMEPAD 0x01
> > #endif /* !SMALL_KERNEL */
> >
> > #define DEVNAME(sc)((sc)->sc_dev.dv_xname)
> > @@ -118,10 +121,10 @@ uhidev_match(struct device *parent, void
> > if (id == NULL)
> > return (UMATCH_NONE);
> > #ifndef SMALL_KERNEL
> > - if (uaa->vendor == USB_VENDOR_MICROSOFT &&
> > - uaa->product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER &&
> > - id->bInterfaceNumber == 0)
> > - return (UMATCH_VENDOR_PRODUCT);
> > + if (id->bInterfaceClass == UICLASS_VENDOR &&
> > + id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> > + id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)
> > + return (UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO);
> > #endif /* !SMALL_KERNEL */
> > if (id->bInterfaceClass != UICLASS_HID)
> > return (UMATCH_NONE);
> > @@ -191,7 +194,7 @@ uhidev_attach(struct device *parent, str
> > }
> >
> > #ifndef SMALL_KERNEL
> > - if (uhidev_use_rdesc(sc, uaa->vendor, uaa->product, &desc, &size))
> > + if (uhidev_use_rdesc(sc, id, uaa->vendor, uaa->product, &desc, &size))
> > return;
> > #endif /* !SMALL_KERNEL */
> >
> > @@ -275,8 +278,8 @@ uhidev_attach(struct device *parent, str
> >
> > #ifndef SMALL_KERNEL
> > int
> > -uhidev_use_rdesc(struct uhidev_softc *sc, int vendor, int product,
> > -void **descp, int *sizep)
> > +uhidev_use_rdesc(struct uhidev_softc *sc, usb_interface_descriptor_t *id,
> > + int vendor, int product, void **descp, int *sizep)
> > {
> > static uByte reportbuf[] = {2, 2};
> > const void *descptr = NULL;
> > @@ -300,8 +303,9 @@ uhidev_use_rdesc(struct uhidev_softc *sc
> > default:
> > break;
> > }
> > - } else if (vendor == USB_VENDOR_MICROSOFT &&
> > - product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER) {
> > + } else if ((id->bInterfaceClass == UICLASS_VENDOR &&
> > + id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> > + id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)) {
> > /* The Xbox 360 gamepad has no report descriptor. */
> > size = sizeof(uhid_xb360gp_report_descr);
> > descptr = uhid_xb360gp_report_descr;
> >
Does grep need a version?
It's been 0.9 since the original import in 2003...
Index: grep.1
===
RCS file: /cvs/src/usr.bin/grep/grep.1,v
retrieving revision 1.43
diff -u -p -r1.43 grep.1
--- grep.1 13 Jan 2015 04:45:34 - 1.43
+++ grep.1 8 Dec 2015 04:59:35 -
@@ -244,9 +244,6 @@ Nonexistent and unreadable files are ign
(i.e. their error messages are suppressed).
.It Fl U
Search binary files, but do not attempt to print them.
-.It Fl V
-Display version information.
-All other options are ignored.
.It Fl v
Selected lines are those
.Em not
Index: grep.c
===
RCS file: /cvs/src/usr.bin/grep/grep.c,v
retrieving revision 1.55
diff -u -p -r1.55 grep.c
--- grep.c 28 Nov 2015 01:17:12 - 1.55
+++ grep.c 8 Dec 2015 04:59:35 -
@@ -137,7 +137,6 @@ static const struct option long_options[
{"basic-regexp",no_argument,NULL, 'G'},
{"with-filename", no_argument,NULL, 'H'},
{"binary", no_argument,NULL, 'U'},
- {"version", no_argument,NULL, 'V'},
{"text",no_argument,NULL, 'a'},
{"byte-offset", no_argument,NULL, 'b'},
{"count", no_argument,NULL, 'c'},
@@ -328,10 +327,6 @@ main(int argc, char *argv[])
break;
case 'U':
binbehave = BIN_FILE_BIN;
- break;
- case 'V':
- fprintf(stderr, "grep version %u.%u\n", VER_MAJ,
VER_MIN);
- exit(0);
break;
#ifndef NOZ
case 'Z':
Index: grep.h
===
RCS file: /cvs/src/usr.bin/grep/grep.h,v
retrieving revision 1.23
diff -u -p -r1.23 grep.h
--- grep.h 7 Dec 2015 18:50:06 - 1.23
+++ grep.h 8 Dec 2015 04:59:35 -
@@ -34,9 +34,6 @@
#include
#include
-#define VER_MAJ 0
-#define VER_MIN 9
-
#define BIN_FILE_BIN 0
#define BIN_FILE_SKIP 1
#define BIN_FILE_TEXT 2
Re: dhclient hang
I think trying again for a different lease is the right thing.. I'm
really tired of dhclient exiting for stupid reasons ;)
On Mon, Dec 7, 2015 at 3:11 PM, Kenneth Westerback
wrote:
> I don't understand what you mean by hang? Does it hang the box? I would
> expect dhclient to reject the offer and try again. Does dhclient never try
> again?
>
> I'll take a closer look tomorrow, but if it isn't now it should be easy to
> fix dhclient to try again for a different lease.
>
> Ken
> On 7 Dec 2015 10:57, "Martin Pieuchot" wrote:
>
>> If for some reason SIOCAIFADDR fails, exit instead of hanging.
>>
>> Without this diff I need to kill dhclient(8) with ctrl+C during boot
>> if I happen to have the address offered by the server configured on
>> a different interface.
>>
>> WARNING: do not try to do that on -current without my rt_delete diffs
>> or your kernel with panic!
>>
>> Ok?
>>
>> Index: kroute.c
>> ===
>> RCS file: /cvs/src/sbin/dhclient/kroute.c,v
>> retrieving revision 1.75
>> diff -u -p -r1.75 kroute.c
>> --- kroute.c10 Feb 2015 04:20:26 - 1.75
>> +++ kroute.c7 Dec 2015 15:51:17 -
>> @@ -473,7 +473,7 @@ priv_add_address(struct imsg_add_address
>> /* No need to set broadcast address. Kernel can figure it out. */
>>
>> if (ioctl(s, SIOCAIFADDR, &ifaliasreq) == -1)
>> - warning("SIOCAIFADDR failed (%s): %s",
>> inet_ntoa(imsg->addr),
>> + error("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
>> strerror(errno));
>>
>> close(s);
>>
Re: dhclient hang
I don't understand what you mean by hang? Does it hang the box? I would
expect dhclient to reject the offer and try again. Does dhclient never try
again?
I'll take a closer look tomorrow, but if it isn't now it should be easy to
fix dhclient to try again for a different lease.
Ken
On 7 Dec 2015 10:57, "Martin Pieuchot" wrote:
> If for some reason SIOCAIFADDR fails, exit instead of hanging.
>
> Without this diff I need to kill dhclient(8) with ctrl+C during boot
> if I happen to have the address offered by the server configured on
> a different interface.
>
> WARNING: do not try to do that on -current without my rt_delete diffs
> or your kernel with panic!
>
> Ok?
>
> Index: kroute.c
> ===
> RCS file: /cvs/src/sbin/dhclient/kroute.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 kroute.c
> --- kroute.c10 Feb 2015 04:20:26 - 1.75
> +++ kroute.c7 Dec 2015 15:51:17 -
> @@ -473,7 +473,7 @@ priv_add_address(struct imsg_add_address
> /* No need to set broadcast address. Kernel can figure it out. */
>
> if (ioctl(s, SIOCAIFADDR, &ifaliasreq) == -1)
> - warning("SIOCAIFADDR failed (%s): %s",
> inet_ntoa(imsg->addr),
> + error("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
> strerror(errno));
>
> close(s);
>
Re: preparing multitouch support - request for tests
Hi, here are the diffs again, as tgz archive (it's the version I posted last week, which doesn't include the latest bug fix: hilms is incomplete in that version and won't work with tablets, only with mice). On 12/07/15 15:59, Matthieu Herrb wrote: On Thu, Dec 03, 2015 at 12:20:15AM +0100, Ulf Brosziewski wrote: The diffs below contain a complete and extensive rewrite of the input-processing parts of wsmouse and the interface it provides to the hardware drivers. It prepares the support for various kinds of multitouch input, as well as an extended support for touchpads by wsmouse. Hi, it looks like the diff got some white space mangled somewhere on the path to my mailbox. Would you have a version available somewhere with a more reliable transport than email ? Thank you. wsmouseinput.tgz Description: Binary data
Re: [PATCH] doas authentication type
On 2015/11/25 00:14, Stuart Henderson wrote:
> On 2015/11/24 11:24, Richard Johnson wrote:
> > We use 2-factor authn for sudo & doas, as well as for most logins.
> > Presently, we transport Yubikey and other HOTP strings across RADIUS to an
> > otpd authserver
>
> Interesting...is that a fork of the TRI-D otpd? I found the googlecode
> one and a github export but nothing that seems currently active and
> nothing that supports Yubikey. (I'm on the lookout for things which
> handles central Yubikey auth, none of the programs that I've found so
> far are very appealing).
>
> > This is on systems with 1200+ user accounts, about 30 active daily. Users
> > expect that if they can log in as username:radius or username:skey, they
> > should be able to sudo -a radius or sudo -a skey.
> >
> > Moving away from Kerberos means possible increasing use of sudo or doas by
> > regular users to run transfer commands to data archives. For this, it would
> > be useful if doas supported "-a skey". Then I could just use doas; the
> > command is otherwise plain enough.
> >
> > But that's not a lot of users across the entire OpenBSD installed base.
> >
> > Installing sudo from ports is still an option. I need to debug the -a
> > failure there now. ;)
> >
> >
> > Richard
> >
>
> Personally my take on this is that as long as it's just done as -a
> then it's small and simple to implement (pass a string from args to
> auth_userokay), and there's no other way to provide access to this which
> is an important, though lesser-known, part of bsd_auth. We already trust
> auth_userokay with network-supplied strings for this (e.g. as part of
> the username from ssh) so this doesn't seem to add any real exposure
> risk.
>
Here's an updated version of Renaud's diff against -current after the change
to auth_userchallenge.
Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.46
diff -u -p -r1.46 doas.c
--- doas.c 3 Dec 2015 08:12:15 - 1.46
+++ doas.c 8 Dec 2015 01:26:19 -
@@ -37,7 +37,8 @@
static void __dead
usage(void)
{
- fprintf(stderr, "usage: doas [-ns] [-C config] [-u user] command
[args]\n");
+ fprintf(stderr, "usage: doas [-ns] [-a style] [-C config] [-u user]"
+ " command [args]\n");
exit(1);
}
@@ -323,6 +324,7 @@ main(int argc, char **argv, char **envp)
int nflag = 0;
char cwdpath[PATH_MAX];
const char *cwd;
+ char *login_style = NULL;
if (pledge("stdio rpath getpw tty proc exec id", NULL) == -1)
err(1, "pledge");
@@ -331,8 +333,11 @@ main(int argc, char **argv, char **envp)
uid = getuid();
- while ((ch = getopt(argc, argv, "C:nsu:")) != -1) {
+ while ((ch = getopt(argc, argv, "a:C:nsu:")) != -1) {
switch (ch) {
+ case 'a':
+ login_style = optarg;
+ break;
case 'C':
confpath = optarg;
break;
@@ -412,7 +417,7 @@ main(int argc, char **argv, char **envp)
if (nflag)
errx(1, "Authorization required");
- if (!(as = auth_userchallenge(myname, NULL, "auth-doas",
+ if (!(as = auth_userchallenge(myname, login_style, "auth-doas",
&challenge)))
err(1, "auth challenge failed");
if (!challenge) {
Index: doas.1
===
RCS file: /cvs/src/usr.bin/doas/doas.1,v
retrieving revision 1.14
diff -u -p -r1.14 doas.1
--- doas.1 27 Jul 2015 17:57:06 - 1.14
+++ doas.1 8 Dec 2015 01:26:19 -
@@ -22,6 +22,7 @@
.Sh SYNOPSIS
.Nm doas
.Op Fl ns
+.Op Fl a Ar style
.Op Fl C Ar config
.Op Fl u Ar user
.Ar command
@@ -40,6 +41,19 @@ is specified.
.Pp
The options are as follows:
.Bl -tag -width tenletters
++.It Fl a Ar style
++The
++.Fl a
++(authentication style) option causes
++.Nm
++to use the specified authentication style when validating the user,
++as allowed by
++.Pa /etc/login.conf .
++The system administrator may specify a list of doas-specific
++authentication methods by adding an
++.Sq auth-doas
++entry in
++.Pa /etc/login.conf .
.It Fl C Ar config
Parse and check the configuration file
.Ar config ,
vlan doesn thave to handle SIOCGIFADDR
cos the stack can do it for it.
ok?
Index: if_vlan.c
===
RCS file: /cvs/src/sys/net/if_vlan.c,v
retrieving revision 1.149
diff -u -p -r1.149 if_vlan.c
--- if_vlan.c 5 Dec 2015 10:07:55 - 1.149
+++ if_vlan.c 8 Dec 2015 01:00:01 -
@@ -599,16 +599,6 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
error = EINVAL;
break;
- case SIOCGIFADDR:
- {
- struct sockaddr *sa;
-
- sa = (struct sockaddr *)&ifr->ifr_data;
- bcopy(((struct arpcom *)ifp->if_softc)->ac_enaddr,
- (caddr_t) sa->sa_data, ETHER_ADDR_LEN);
- }
- break;
-
case SIOCSIFMTU:
if (ifv->ifv_p != NULL) {
if (ifr->ifr_mtu < ETHERMIN ||
Re: preparing multitouch support - request for tests
I believe your remarks and questions concern crucial points, which probably deserve some discussion and careful examination. Please note that behind the extension of the interface that wsmouse provides to the hardware drivers, there is an approach that is somewhat different from, e.g., what Linux does. And no, I don't claim that it makes sense to compare that directly, Linux does a lot of things that we cannot do or don't want to do, but we must have at look it when considering ways of porting libinput. Linux' input module also provides a variety of state- reporting functions and "SYNC" events and functions that "finish" frames. Most or all of the state-reporting functions translate the input into evdev events, sometimes with trivial, sometimes with quite complex additional processing, but it usually ends with generating one or more of those events. wsmouseinput plainly uses structures that are tailored to the different types of state, and records pending changes in their 'sync' flags. No events are generated until wsmouse_input_sync() is called. This makes it easy to apply transformations to the state, and all of them can be part of the sync-operation (there is the conversion of MT input from touchpads into a single-touch representation; for touchpads in compat mode, there is a conversion of the single-touch state(s) into a mouse-compatible state, and the touchpad extension is, strictly speaking, just an addition to these transformations). evdev events have a type, you can query evdev devices for their capabilities, for the range of values that can occur in events of a given type, you can filter events, and presumably do a lot of other things. And libinput seems to be deeply rooted in evdev. I'm not sure whether it would be possible to cleanly separate all that evdev-specific stuff in a single layer and simply use the rest as it is, and/or to add sufficiently evdev-like features to our device and event system. Of course maintaining an in-kernel touchpad driver is a burden and obligation, but it could still be easier to support a thin libinput layer with it than adapting everything else to a "full" libinput port. On 12/07/2015 03:47 AM, joshua stein wrote: On Thu, 03 Dec 2015 at 00:20:15 +0100, Ulf Brosziewski wrote: The diffs below contain a complete and extensive rewrite of the input-processing parts of wsmouse and the interface it provides to the hardware drivers. It prepares the support for various kinds of multitouch input, as well as an extended support for touchpads by wsmouse. It's hard to comment on the diff as a whole, so I'd recommend breaking it up into smaller pieces that can be reviewed and committed independently of each other. Also, please provide some explanation of each change since it's not immediately clear from just the code diffs why you are changing certain things. From my initial reading, it looks like you are primarily changing wsmouse_input to a macro that now calls separate functions for maintaining state about buttons, motion, and pointer placement. I think that's a good change. I don't really agree with the direction of the trackpad driver, though. It looks that you want to move from a "dumb" kernel interface and a "smart" xorg driver (synaptics) to a smart kernel interface and a dumb xorg driver (ws). Aside from wsmoused being able to use it, what is the benefit of putting all the logic in the kernel? I've needed to tweak synclient settings on many of my laptops like scroll speed, multi-finger clicks, palm detection, finger widths. Will all of that still be possible with your kernel driver? I know people hate libinput, but I would prefer to keep the dumb kernel interface and be able to feed slot data to libinput to let those upper layers handle all of the fancy logic and configurability. That way we're not having to re-invent complicated things like accurate palm detection in the kernel, we can keep updating libinput from an upstream (free labor), and we'll be able to take advantage of libinput support in things like gtk+3.
Re: ksh(1): utf8 in emacs editing mode
Hi,
i'd like to propose a simplified version of this patch Frederic Nowak
posted a few weeks ago for commit. Our experience is probably
not yet sufficient to develop a full-blown solution for all UTF-8
problems in ksh(1), but this is very non-intrusive and makes the
following commands better without breaking anything:
* Ctrl-b, Ctrl-xd, Esc-[D, Esc-OD (backward-char)
* Ctrl-f, Ctrl-xc, Esc-[C, Esc-OC (forward-char)
* Ctrl-h, Ctrl-? (delete-char-backward)
* Esc-[3~ (delete-char-forward)
* Ctrl-k (kill-to-eol)
This patch deliberately does not attempt any character counting
or display column counting. It's purely about not chopping UTF-8
characters into pieces, and it is not comprehensive. It only
improves some of the most common cases.
Even though i'm not planning to do more work in ksh(1) myself just
yet, i would not like to see effort wasted that other people are
spending.
See my version of the patch at the end. Before that, i'm commenting
on some parts of Frederic's patch. Note that i'm cutting away the
parts i agree with.
Opinions? Concerns? OKs?
Ingo
Frederic Nowak wrote on Tue, Nov 10, 2015 at 01:36:02PM +0100:
> I got annoyed with ksh(1) for messing up my command line after
> accidentally entering an umlaut and decided to take a stab at teaching
> it some utf8. The diff is inspired by Ted Unangst's recent patches for
> e.g. rs[0].
>
> It works for my use cases and seems to handle 2-byte and 3-byte
> sequences quite well; I hope it does so for longer ones, too. Maybe
> it's of use for someone else as well.
>
> Cheers,
> Frederic
>
> [0] https://marc.info/?l=openbsd-tech&m=144560099607564
>
>
> Index: emacs.c
> ===
> RCS file: /cvs/src/bin/ksh/emacs.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 emacs.c
> --- emacs.c 19 Oct 2015 14:42:16 - 1.60
> +++ emacs.c 10 Nov 2015 12:31:27 -
> @@ -49,7 +49,7 @@ struct x_ftab {
> #define is_cfs(c) (c == ' ' || c == '\t' || c == '"' || c == '\'')
>
> /* Separator for motion */
> -#define is_mfs(c) (!(isalnum((unsigned char)c) || c == '_' || c
> == '$'))
> +#define is_mfs(c) (!(isu8lead((unsigned char) c) ||
> isu8cont((unsigned char) c) || isalnum((unsigned char)c) || c == '_' || c ==
> '$'))
In the first step, i'd like to focus on the simplest commands,
so i'm not including word handling for now.
If we get the first step committed, please consider rebasing
the rest of your work and resubmitting a clean patch.
> /* Arguments for do_complete()
> * 0 = enumerate M-= complete as much as possible and then list
> @@ -198,6 +198,10 @@ static int x_comment(int);
> static int x_debug_info(int);
> #endif
>
> +/* utf8 support */
> +static int isu8cont(unsigned char);
> +static int isu8lead(unsigned char);
> +
Sometimes, it may be needed to detect lead bytes, but in patches
of this style, i'd like to avoid that if possible, because typically,
that keeps the code simpler. In the case at hand, it's possible
without isu8lead().
[...]
> @@ -468,6 +491,8 @@ x_del_back(int c)
> }
> if (x_arg > col)
> x_arg = col;
> + while(x_arg <= col && isu8cont(*(xcp - x_arg)))
> + x_arg++;
> x_goto(xcp - x_arg);
> x_delete(x_arg, false);
> return KSTD;
This seems off by one to me.
If x_arg == col, we must not increment it further, right?
Otherwise, we will access xcp[-1].
Of course, that's a pathological case, because it only happens
when the command line starts with a continuation byte, but still...
> @@ -621,7 +646,7 @@ x_fword(void)
> static void
> x_goto(char *cp)
> {
> - if (cp < xbp || cp >= (xbp + x_displen)) {
> + if (cp < xbp || cp >= xlp) {
> /* we are heading off screen */
> xcp = cp;
> x_adjust();
I don't understand why you propose to change this,
nor how it's related to UTF-8.
[...]
> @@ -669,7 +696,8 @@ x_zots(char *str)
> int adj = x_adj_done;
>
> x_lastcp();
> - while (*str && str < xlp && adj == x_adj_done)
> + while (*str && (isu8cont(*str) || str < xlp)
> +&& adj == x_adj_done)
> x_zotc(*str++);
> }
Isn't that change redundant?
Given that x_size() returns 0 for continuation bytes,
x_lastcp() can never leave xlp on a continuation byte,
or can it?
> @@ -697,6 +725,8 @@ x_mv_back(int c)
> }
> if (x_arg > col)
> x_arg = col;
> + while(x_arg <= col && isu8cont(*(xcp - x_arg)))
> + x_arg++;
> x_goto(xcp - x_arg);
> return KSTD;
> }
That looks like off by one to me, too.
> @@ -710,6 +740,7 @@ x_mv_forw(int c)
> x_e_putc(BEL);
> return KSTD;
> }
> + x_arg += isu8lead(*xcp);
> if (x_arg > nleft)
> x_arg = nleft;
> x_goto(xcp + x_arg);
I suggest to look at the next byte and use isu8cont(),
see the patch below.
> @@ -1025,7
patch 4/4 add keyboard backlight support to asmc(4)
Hi,
here comes another diff of the series for the keyboard backlight
support.
Please find below a diff which enables keyboard backlight control on
Intel Apple Laptops via asmc(4).
This diff uses introduced wskbd(4) hooks from previously sent diffs and
also introduces locking and minor refactoring around asmc_try().
Comments, tests, OKs?
Thanks,
Regards,
Joerg
Index: asmc.c
===
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.13
diff -u -p -r1.13 asmc.c
--- asmc.c 29 Oct 2015 13:29:04 - 1.13
+++ asmc.c 7 Dec 2015 22:50:02 -
@@ -23,12 +23,14 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
#define ASMC_BASE 0x300 /* SMC base address */
#define ASMC_IOSIZE32 /* I/O region size 0x300-0x31f */
@@ -66,10 +68,13 @@ struct asmc_softc {
uint8_t sc_init; /* initialization done? */
uint8_t sc_nfans; /* number of fans */
uint8_t sc_lightlen; /* light data len */
+ uint8_t sc_kbdled; /* backlight led value */
+ struct rwlocksc_lock;
struct taskq*sc_taskq;
struct task sc_task_init;
struct task sc_task_refresh;
+ struct task sc_task_backlight;
struct ksensor sc_sensor_temp[ASMC_MAXTEMP];
struct ksensor sc_sensor_fan[ASMC_MAXFAN];
@@ -81,7 +86,6 @@ struct asmc_softc {
uint8_tasmc_status(struct asmc_softc *);
intasmc_try(struct asmc_softc *, int, const char *, uint8_t *, uint8_t);
-void asmc_kbdled(struct asmc_softc *, uint8_t);
void asmc_init(void *);
void asmc_refresh(void *);
@@ -91,6 +95,13 @@ int asmc_match(struct device *, void *,
void asmc_attach(struct device *, struct device *, void *);
intasmc_detach(struct device *, int);
+/* wskbd hook functions */
+void asmc_kbdled(void *);
+intasmc_get_backlight(struct wskbd_backlight *);
+intasmc_set_backlight(struct wskbd_backlight *);
+extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
+extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
+
const struct cfattach asmc_ca = {
sizeof(struct asmc_softc), asmc_match, asmc_attach
};
@@ -256,7 +267,7 @@ asmc_attach(struct device *parent, struc
struct asmc_softc *sc = (struct asmc_softc *)self;
struct isa_attach_args *ia = aux;
uint8_t buf[6];
- int i;
+ int i, r;
if (bus_space_map(ia->ia_iot, ia->ia_iobase, ia->ia_iosize, 0,
&sc->sc_ioh)) {
@@ -265,23 +276,34 @@ asmc_attach(struct device *parent, struc
}
sc->sc_iot = ia->ia_iot;
- if (asmc_try(sc, ASMC_READ, "REV ", buf, 6)) {
- printf(": revision failed (0x%x)\n", asmc_status(sc));
+ rw_init(&sc->sc_lock, sc->sc_dev.dv_xname);
+
+ if ((r = asmc_try(sc, ASMC_READ, "REV ", buf, 6))) {
+ printf(": revision failed (0x%x)\n", r);
bus_space_unmap(ia->ia_iot, ia->ia_iobase, ASMC_IOSIZE);
return;
}
printf(": rev %x.%x%x%x", buf[0], buf[1], buf[2],
ntohs(*(uint16_t *)buf + 4));
- if (asmc_try(sc, ASMC_READ, "#KEY", buf, 4)) {
- printf(", no of keys failed (0x%x)\n", asmc_status(sc));
+ if ((r = asmc_try(sc, ASMC_READ, "#KEY", buf, 4))) {
+ printf(", no of keys failed (0x%x)\n", r);
bus_space_unmap(ia->ia_iot, ia->ia_iobase, ASMC_IOSIZE);
return;
}
printf(", %u key%s\n", ntohl(*(uint32_t *)buf),
(ntohl(*(uint32_t *)buf) == 1) ? "" : "s");
- asmc_kbdled(sc, 127);
+ /* keyboard backlight led is optional */
+ sc->sc_kbdled = buf[0] = 127, buf[1] = 0;
+ if ((r = asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2))) {
+ if (r != ASMC_NOTFOUND)
+ printf("%s: keyboard backlight failed (0x%x)\n",
+ sc->sc_dev.dv_xname, r);
+ } else {
+ wskbd_get_backlight = asmc_get_backlight;
+ wskbd_set_backlight = asmc_set_backlight;
+ }
if (!(sc->sc_taskq = taskq_create("asmc", 1, IPL_NONE, 0))) {
printf("%s: can't create task queue\n", sc->sc_dev.dv_xname);
@@ -290,6 +312,7 @@ asmc_attach(struct device *parent, struc
}
task_set(&sc->sc_task_init, asmc_init, sc);
task_set(&sc->sc_task_refresh, asmc_refresh, sc);
+ task_set(&sc->sc_task_backlight, asmc_kbdled, sc);
strlcpy(sc->sc_sensor_dev.xname, sc->sc_dev.dv_xname,
sizeof(sc->sc_sensor_dev.xname));
@@ -327,6 +350,7 @@ int
asmc_detach(struct device *self, int flags)
{
struct asmc_softc *sc = (struct asmc_softc *)self;
+ uint8_t buf[2] = { (sc->sc_kbdled = 0), 0 };
patch 3/4 add generic keyboard backlight support
Hi,
here comes the third part of the series for generic keyboard backlight
support.
Please find below a diff which adds they key(code)s for keyboard
backlight control, as found on all recent Intel based Apple Laptop
Keyboards. While here, also add keys for display brightness control
found on Apple Keyboards as well.
This is based on a similar diff from Sven-Volker Nowarra and is a NOOP
for now, as these keys are not used yet. Using them is target for a
later diff.
I'm not familar with keycodes, most of the diff below is 'educated
guess', so please, hints are welcome!
Comments, OK?
Thanks,
Regards,
Joerg
Index: pckbc/wskbdmap_mfii.c
===
RCS file: /cvs/src/sys/dev/pckbc/wskbdmap_mfii.c,v
retrieving revision 1.43
diff -u -p -r1.43 wskbdmap_mfii.c
--- pckbc/wskbdmap_mfii.c 14 Apr 2013 19:32:52 - 1.43
+++ pckbc/wskbdmap_mfii.c 7 Dec 2015 22:22:27 -
@@ -149,6 +149,10 @@ static const keysym_t pckbd_keydesc_us[]
KC(170), KS_Print_Screen,
KC(174), KS_AudioLower,
KC(176), KS_AudioRaise,
+KC(177), KS_BrightnessDown,
+KC(178), KS_BrightnessUp,
+KC(179), KS_KbdBacklightDown,
+KC(180), KS_KbdBacklightUp,
KC(181), KS_KP_Divide,
KC(183), KS_Print_Screen,
KC(184), KS_Cmd2, KS_Alt_R, KS_Multi_key,
Index: wscons/wsksymdef.h
===
RCS file: /cvs/src/sys/dev/wscons/wsksymdef.h,v
retrieving revision 1.36
diff -u -p -r1.36 wsksymdef.h
--- wscons/wsksymdef.h 26 Jan 2014 17:48:08 - 1.36
+++ wscons/wsksymdef.h 7 Dec 2015 22:22:27 -
@@ -633,6 +633,10 @@
#define KS_AudioMute 0xf3d1
#define KS_AudioLower 0xf3d2
#define KS_AudioRaise 0xf3d3
+#define KS_BrightnessDown 0xf3d4
+#define KS_BrightnessUp0xf3d5
+#define KS_KbdBacklightDown0xf3d6
+#define KS_KbdBacklightUp 0xf3d7
/*
* Group 4 (command)
Index: usb/makemap.awk
===
RCS file: /cvs/src/sys/dev/usb/makemap.awk,v
retrieving revision 1.14
diff -u -p -r1.14 makemap.awk
--- usb/makemap.awk 20 Nov 2013 17:27:32 - 1.14
+++ usb/makemap.awk 7 Dec 2015 22:22:27 -
@@ -153,6 +153,10 @@ BEGIN {
conv[170] = 70
conv[174] = 129
conv[176] = 128
+ conv[177] = 131
+ conv[178] = 132
+ conv[179] = 133
+ conv[180] = 134
conv[181] = 84
conv[184] = 230
# 198 is #if 0 in the PS/2 map...
Index: usb/ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.71
diff -u -p -r1.71 ukbd.c
--- usb/ukbd.c 14 Mar 2015 03:38:50 - 1.71
+++ usb/ukbd.c 7 Dec 2015 22:22:27 -
@@ -469,13 +469,15 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
static const struct ukbd_translation apple_fn_trans[] = {
{ 40, 73 }, /* return -> insert */
{ 42, 76 }, /* backspace -> delete */
+ { 58, 131 },/* F1 -> screen brightness down */
+ { 59, 132 },/* F2 -> screen brightness up */
#ifdef notyet
- { 58, 0 }, /* F1 -> screen brightness down */
- { 59, 0 }, /* F2 -> screen brightness up */
{ 60, 0 }, /* F3 */
{ 61, 0 }, /* F4 */
- { 62, 0 }, /* F5 -> keyboard backlight down */
- { 63, 0 }, /* F6 -> keyboard backlight up */
+#endif
+ { 62, 133 },/* F5 -> keyboard backlight down */
+ { 63, 134 },/* F6 -> keyboard backlight up */
+#ifdef notyet
{ 64, 0 }, /* F7 -> audio back */
{ 65, 0 }, /* F8 -> audio pause/play */
{ 66, 0 }, /* F9 -> audio next */
Index: usb/ukbdmap.c
===
RCS file: /cvs/src/sys/dev/usb/ukbdmap.c,v
retrieving revision 1.41
diff -u -p -r1.41 ukbdmap.c
--- usb/ukbdmap.c 20 Nov 2013 17:28:00 - 1.41
+++ usb/ukbdmap.c 7 Dec 2015 22:22:27 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: ukbdmap.c,v 1.41 2013/11/20 17:28:00 miod Exp $ */
+/* $OpenBSD$ */
/*
* THIS FILE IS AUTOMAGICALLY GENERATED. DO NOT EDIT.
@@ -176,6 +176,10 @@ static const keysym_t ukbd_keydesc_us[]
KC(127), KS_AudioMute,
KC(128), KS_AudioRaise,
KC(129), KS_AudioLower,
+KC(131), KS_BrightnessDown,
+KC(132), KS_BrightnessUp,
+KC(133), KS_KbdBacklightDown,
+KC(134), KS_KbdBacklightUp,
KC(224), KS_Cmd1,KS_Control_L,
KC(225), KS_Shift_L,
KC(226), KS_Cmd2,KS_Alt_L,
patch 1/4 add generic keyboard backlight support
Hi,
here comes a series of small diffs which add generic support for
keyboard backlights.
Please find below the first diff, which adds new ioctls to wskbd(4) to
control keyboard backlights.
In contrast to an earlier diff from jcs, I have chosen to use a struct
in favor of a simple (unsigned) int as depending on the vendor
(Thinkpad, Apple, ...) different min/max values for the brightness of
the keyboard backlight are possible.
Comments, OK?
Thanks,
Regards,
Joerg
Index: wsconsio.h
===
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.72
diff -u -p -r1.72 wsconsio.h
--- wsconsio.h 30 Aug 2015 10:05:09 - 1.72
+++ wsconsio.h 7 Dec 2015 21:03:45 -
@@ -180,6 +180,13 @@ struct wskbd_map_data {
#define WSKBDIO_GETENCODING_IOR('W', 15, kbd_t)
#define WSKBDIO_SETENCODING_IOW('W', 16, kbd_t)
+/* Get/set keyboard backlight. Not applicable to all keyboard types. */
+struct wskbd_backlight {
+ unsigned int min, max, curval;
+};
+#defineWSKBDIO_GETBACKLIGHT_IOR('W', 17, struct wskbd_backlight)
+#defineWSKBDIO_SETBACKLIGHT_IOW('W', 18, struct wskbd_backlight)
+
/* internal use only */
#define WSKBDIO_SETMODE_IOW('W', 19, int)
#define WSKBDIO_GETMODE_IOR('W', 20, int)
Index: wskbd.c
===
RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
retrieving revision 1.82
diff -u -p -r1.82 wskbd.c
--- wskbd.c 10 Sep 2015 18:14:52 - 1.82
+++ wskbd.c 7 Dec 2015 21:03:45 -
@@ -230,6 +230,9 @@ int wskbd_mux_close(struct wsevsrc *);
intwskbd_do_open(struct wskbd_softc *, struct wseventvar *);
intwskbd_do_ioctl(struct device *, u_long, caddr_t, int, struct proc *);
+int(*wskbd_get_backlight)(struct wskbd_backlight *);
+int(*wskbd_set_backlight)(struct wskbd_backlight *);
+
struct cfdriver wskbd_cd = {
NULL, "wskbd", DV_TTY
};
@@ -1010,6 +1013,7 @@ wskbd_displayioctl(struct device *dev, u
case WSKBDIO_SETDEFAULTKEYREPEAT:
case WSKBDIO_SETMAP:
case WSKBDIO_SETENCODING:
+ case WSKBDIO_SETBACKLIGHT:
if ((flag & FWRITE) == 0)
return (EACCES);
}
@@ -1155,6 +1159,18 @@ getkeyrepeat:
wsmux_set_layout(sc->sc_base.me_parent, enc);
#endif
return (0);
+
+ case WSKBDIO_GETBACKLIGHT:
+ if (wskbd_get_backlight != NULL)
+ return (*wskbd_get_backlight)((struct wskbd_backlight
*)data);
+ error = ENOTTY;
+ break;
+
+ case WSKBDIO_SETBACKLIGHT:
+ if (wskbd_set_backlight != NULL)
+ return (*wskbd_set_backlight)((struct wskbd_backlight
*)data);
+ error = ENOTTY;
+ break;
}
/*
patch 2/4 add generic keyboard backlight support
Hi,
here comes the second part of the series for generic keyboard backlight
support.
Please find below the userland part which adds a backlight variable to
wsconsctl(8).
I have chosen to use the percentage format, so it looks like this:
$ doas wsconsctl keyboard.backlight=80
keyboard.backlight -> 80.00%
Comments, OK?
Thanks,
Regards,
Joerg
Index: keyboard.c
===
RCS file: /cvs/src/sbin/wsconsctl/keyboard.c,v
retrieving revision 1.12
diff -u -p -r1.12 keyboard.c
--- keyboard.c 21 Mar 2013 06:04:05 - 1.12
+++ keyboard.c 7 Dec 2015 21:03:22 -
@@ -50,6 +50,7 @@ static struct wskbd_keyrepeat_data repea
static struct wskbd_keyrepeat_data dfrepeat;
static int ledstate;
static kbd_t kbdencoding;
+static struct field_pc backlight;
struct field keyboard_field_tab[] = {
{ "type", &kbtype,FMT_KBDTYPE,FLG_RDONLY },
@@ -66,12 +67,15 @@ struct field keyboard_field_tab[] = {
{ "repeat.deln.default", &dfrepeat.delN, FMT_UINT, FLG_MODIFY },
{ "ledstate", &ledstate, FMT_UINT, 0 },
{ "encoding", &kbdencoding, FMT_KBDENC, FLG_MODIFY },
+{ "backlight", &backlight, FMT_PC,
FLG_MODIFY|FLG_INIT },
{ NULL }
};
void
keyboard_get_values(int fd)
{
+ struct wskbd_backlight kbl;
+
if (field_by_value(keyboard_field_tab, &kbtype)->flags & FLG_GET)
if (ioctl(fd, WSKBDIO_GTYPE, &kbtype) < 0)
warn("WSKBDIO_GTYPE");
@@ -131,11 +135,24 @@ keyboard_get_values(int fd)
if (field_by_value(keyboard_field_tab, &kbdencoding)->flags & FLG_GET)
if (ioctl(fd, WSKBDIO_GETENCODING, &kbdencoding) < 0)
warn("WSKBDIO_GETENCODING");
+
+ if (field_by_value(keyboard_field_tab, &backlight)->flags & FLG_GET) {
+ if (ioctl(fd, WSKBDIO_GETBACKLIGHT, &kbl) < 0)
+ warn("WSKBDIO_GETBACKLIGHT");
+ else {
+backlight.min = kbl.min;
+backlight.cur = kbl.curval;
+backlight.max = kbl.max;
+ }
+ }
+
}
int
keyboard_put_values(int fd)
{
+ struct wskbd_backlight kbl;
+
bell.which = 0;
if (field_by_value(keyboard_field_tab, &bell.pitch)->flags & FLG_SET)
bell.which |= WSKBD_BELL_DOPITCH;
@@ -200,6 +217,16 @@ keyboard_put_values(int fd)
if (field_by_value(keyboard_field_tab, &kbdencoding)->flags & FLG_SET) {
if (ioctl(fd, WSKBDIO_SETENCODING, &kbdencoding) < 0) {
warn("WSKBDIO_SETENCODING");
+ return 1;
+ }
+ }
+
+ if (field_by_value(keyboard_field_tab, &backlight)->flags & FLG_SET) {
+ kbl.min = backlight.min;
+ kbl.curval = backlight.cur;
+ kbl.max = backlight.max;
+ if (ioctl(fd, WSKBDIO_SETBACKLIGHT, &kbl) < 0) {
+ warn("WSKBDIO_SETBACKLIGHT");
return 1;
}
}
Re: [patch] fix urtw on big-endian CPU (PPC)
On Mon, Dec 07, 2015 at 09:31:11PM +0100, Cédric TESSIER wrote: > Hi, > > I'm bringing my old Mac Mini G4 back to life, and I had an issue with a > wireless dongle (RTL8187 rev 0x04, RFv2). > > urtw interface was available, but wasn't working at all. > > Investigations highlighted an endianness related issue. > > I've written a quick and dirty patch which make the interface fully working > again. > > I've tested it on PPC and i386 (5.8 and snapshot). > > > Cédric Thanks. I do have the necessary hardware to test this. I'll try to get that done this week and get your fix committed. > > > > --- if_urtw.c.orig2015-11-25 04:10:00.0 +0100 > +++ if_urtw.c 2015-12-07 11:40:05.0 +0100 > @@ -1078,6 +1078,7 @@ > USETW(req.wIndex, index); > USETW(req.wLength, sizeof(uint16_t)); > > + data = htole16(data); > return (usbd_do_request(sc->sc_udev, &req, &data)); > } > > @@ -1587,6 +1588,7 @@ > USETW(req.wLength, sizeof(uint16_t)); > > error = usbd_do_request(sc->sc_udev, &req, data); > + *data = letoh16(*data); > return (error); > } > > @@ -1603,6 +1605,7 @@ > USETW(req.wLength, sizeof(uint32_t)); > > error = usbd_do_request(sc->sc_udev, &req, data); > + *data = letoh32(*data); > return (error); > } > > @@ -1645,6 +1648,7 @@ > USETW(req.wIndex, idx & 0x03); > USETW(req.wLength, sizeof(uint16_t)); > > + data = htole16(data); > return (usbd_do_request(sc->sc_udev, &req, &data)); > } > > @@ -1659,6 +1663,7 @@ > USETW(req.wIndex, idx & 0x03); > USETW(req.wLength, sizeof(uint32_t)); > > + data = htole32(data); > return (usbd_do_request(sc->sc_udev, &req, &data)); > } > >
[patch] mailwrapper: remove broken fallback code
If /etc/mailer.conf doesn't exist, mailwrapper tries to run sendmail,
giving a confusing error message:
mailwrapper: cannot exec /usr/libexec/sendmail/sendmail: No such
file or directory
This patch removes this fallback code. I believe this is cleaner than
updating the fallback since we would have to put two paths in: one for
sendmail/send-mail/mailq and one for makemap/newaliases.
Index: mailwrapper.c
===
RCS file: /cvs/src/usr.sbin/mailwrapper/mailwrapper.c,v
retrieving revision 1.20
diff -u -p -r1.20 mailwrapper.c
--- mailwrapper.c 12 Oct 2015 22:01:08 - 1.20
+++ mailwrapper.c 7 Dec 2015 21:33:57 -
@@ -36,12 +36,10 @@
#include
#include
#include
-#include
#include
#include
#define _PATH_MAILERCONF "/etc/mailer.conf"
-#define _PATH_DEFAULTMTA "/usr/libexec/sendmail/sendmail"
struct arglist {
size_t argc, maxc;
@@ -100,21 +98,11 @@ main(int argc, char *argv[], char *envp[
for (len = 0; len < argc; len++)
addarg(&al, argv[len], 0);
- config = fopen(_PATH_MAILERCONF, "r");
+ if ((config = fopen(_PATH_MAILERCONF, "r")) == NULL)
+ err(1, "cannot open %s", _PATH_MAILERCONF);
if (pledge("stdio exec", NULL) == -1)
err(1, "pledge");
-
- if (config == NULL) {
- addarg(&al, NULL, 0);
- openlog(__progname, LOG_PID, LOG_MAIL);
- syslog(LOG_INFO, "cannot open %s, using %s as default MTA",
- _PATH_MAILERCONF, _PATH_DEFAULTMTA);
- closelog();
- execve(_PATH_DEFAULTMTA, al.argv, envp);
- err(1, "cannot exec %s", _PATH_DEFAULTMTA);
- /*NOTREACHED*/
- }
for (;;) {
if ((line = fparseln(config, &len, &lineno, NULL, 0)) == NULL) {
Re: Do not pass NULL to rtdeletemsg()
On 12/07/15 14:57, Martin Pieuchot wrote:
> If the interface is gone that means you're dealing with a cached route
> so there's no need to try to remove it from the table.
>
> Better be explicit and do that before calling rtdeletemsg() rather than
> inside.
>
> ok?
ok vgross@
>
> Index: netinet/ip_icmp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 ip_icmp.c
> --- netinet/ip_icmp.c 3 Dec 2015 21:11:53 - 1.150
> +++ netinet/ip_icmp.c 7 Dec 2015 12:40:06 -
> @@ -1042,19 +1042,21 @@ icmp_mtudisc(struct icmp *icp, u_int rta
> void
> icmp_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
> {
> - if (rt == NULL)
> - panic("icmp_mtudisc_timeout: bad route to timeout");
> + struct ifnet *ifp;
> + int s;
>
> - if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_DYNAMIC | RTF_HOST)) {
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
> +
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
> void *(*ctlfunc)(int, struct sockaddr *, u_int, void *);
> struct sockaddr_in sin;
> - int s;
>
> sin = *satosin(rt_key(rt));
>
> s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
>
> /* Notify TCP layer of increased Path MTU estimate */
> ctlfunc = inetsw[ip_protox[IPPROTO_TCP]].pr_ctlinput;
> @@ -1062,9 +1064,12 @@ icmp_mtudisc_timeout(struct rtentry *rt,
> (*ctlfunc)(PRC_MTUINC, sintosa(&sin),
> r->rtt_tableid, NULL);
> splx(s);
> - } else
> + } else {
> if ((rt->rt_rmx.rmx_locks & RTV_MTU) == 0)
> rt->rt_rmx.rmx_mtu = 0;
> + }
> +
> + if_put(ifp);
> }
>
> /*
> @@ -1088,17 +1093,20 @@ icmp_ratelimit(const struct in_addr *dst
> void
> icmp_redirect_timeout(struct rtentry *rt, struct rttimer *r)
> {
> - if (rt == NULL)
> - panic("icmp_redirect_timeout: bad route to timeout");
> + struct ifnet *ifp;
> + int s;
>
> - if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_DYNAMIC | RTF_HOST)) {
> - int s;
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
>
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
> s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
> splx(s);
> }
> +
> + if_put(ifp);
> }
>
> int
> Index: netinet6/icmp6.c
> ===
> RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> retrieving revision 1.182
> diff -u -p -r1.182 icmp6.c
> --- netinet6/icmp6.c 3 Dec 2015 21:11:53 - 1.182
> +++ netinet6/icmp6.c 7 Dec 2015 12:39:28 -
> @@ -1952,34 +1952,42 @@ icmp6_mtudisc_clone(struct sockaddr *dst
> void
> icmp6_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
> {
> - if (rt == NULL)
> - panic("icmp6_mtudisc_timeout: bad route to timeout");
> - if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_DYNAMIC | RTF_HOST)) {
> - int s;
> + struct ifnet *ifp;
> + int s;
>
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
> +
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
> s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
> splx(s);
> } else {
> if (!(rt->rt_rmx.rmx_locks & RTV_MTU))
> rt->rt_rmx.rmx_mtu = 0;
> }
> +
> + if_put(ifp);
> }
>
> void
> icmp6_redirect_timeout(struct rtentry *rt, struct rttimer *r)
> {
> - if (rt == NULL)
> - panic("icmp6_redirect_timeout: bad route to timeout");
> - if ((rt->rt_flags & (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) ==
> - (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) {
> - int s;
> + struct ifnet *ifp;
> + int s;
>
> + ifp = if_get(rt->rt_ifidx);
> + if (ifp == NULL)
> + return;
> +
> + if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
> s = splsoftnet();
> - rtdeletemsg(rt, NULL, r->rtt_tableid);
> + rtdeletemsg(rt, ifp, r->rtt_tableid);
> splx(s);
> }
> +
> + if_put(ifp);
> }
>
> int *icmpv6ctl_vars[ICMPV6CTL_MAXID] = ICMPV6CTL_VARS;
>
Re: [patch] Convert modulus to arc4random_uniform
> I'll look into hack tonight when I have more time.
Honestly, I would prefer to leave hack as it is right now since it will
take some work to repair it anyway. I would not want to add another
layer of (potential) complications.
> > > Index: lib/libc/stdlib/rand.c
> > > ===
> It's safe but takes a bit of thinking. I first had it as
> return (arc4random() & RAND_MAX);
> which to me is more obviously correct, but since it's safe as is. I have
> no strong opinion on this.
I have a mild preference for this version, and I think this version
would be preferable from the point of view of uniformity of usage,
especially after your patches have gone in.
> > > Index: usr.bin/awk/run.c
> > > ===
> >
> > Unsure about this one. I think deterministic sequences might be desired
> > in some circumstances (this one is deterministic when a seed was given).
>
> theo@ also pointed out that awk can be deterministic. Since RAND_MAX is
> 1 below a power of 2, & is safe. How about
>
> Index: run.c
> ===
> RCS file: /cvs/src/usr.bin/awk/run.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 run.c
> --- run.c 5 Sep 2015 22:07:10 - 1.39
> +++ run.c 7 Dec 2015 19:28:31 -
> @@ -1581,7 +1581,7 @@ Cell *bltin(Node **a, int n)/* builtin
> u = (Awkfloat) system(getsval(x)) / 256; /* 256 is unix-dep */
> break;
> case FRAND:
> - u = (Awkfloat) (random() % RAND_MAX) / RAND_MAX;
> + u = (Awkfloat) (random() & RAND_MAX) / ((u_int)RAND_MAX + 1);
> break;
> case FSRAND:
> if (isrec(x)) { /* no argument provided */
>
ok from me on this one.
Re: [vi] Remove needless (m|c)alloc aliases
On Mon, Dec 07, 2015 at 03:44:12PM -0500, Michael McConville wrote: > No binary change. ok? ok tb@
[vi] Remove needless (m|c)alloc aliases
No binary change. ok?
Index: cl/cl_main.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.27
diff -u -p -r1.27 cl_main.c
--- cl/cl_main.c7 Dec 2015 20:39:19 - 1.27
+++ cl/cl_main.c7 Dec 2015 20:41:43 -
@@ -151,7 +151,7 @@ gs_init(char *name)
name = p + 1;
/* Allocate the global structure. */
- CALLOC_NOMSG(NULL, gp, 1, sizeof(GS));
+ gp = calloc(1, sizeof(GS));
if (gp == NULL)
perr(name, NULL);
@@ -171,7 +171,7 @@ cl_init(GS *gp)
int fd;
/* Allocate the CL private structure. */
- CALLOC_NOMSG(NULL, clp, 1, sizeof(CL_PRIVATE));
+ clp = calloc(1, sizeof(CL_PRIVATE));
if (clp == NULL)
perr(gp->progname, NULL);
gp->cl_private = clp;
Index: common/main.c
===
RCS file: /cvs/src/usr.bin/vi/common/main.c,v
retrieving revision 1.31
diff -u -p -r1.31 main.c
--- common/main.c 7 Dec 2015 20:39:19 - 1.31
+++ common/main.c 7 Dec 2015 20:41:43 -
@@ -360,7 +360,7 @@ editor(GS *gp, int argc, char *argv[])
size_t l;
/* Cheat -- we know we have an extra argv slot. */
l = strlen(sp->frp->name) + 1;
- MALLOC_NOMSG(sp, *--argv, l);
+ *--argv = malloc(l);
if (*argv == NULL) {
v_estr(gp->progname, errno, NULL);
goto err;
@@ -563,7 +563,7 @@ v_obsolete(char *name, char *argv[])
} else {
p = argv[0];
len = strlen(argv[0]);
- MALLOC_NOMSG(NULL, argv[0], len + 2);
+ argv[0] = malloc(len + 2);
if (argv[0] == NULL)
goto nomem;
argv[0][0] = '-';
Index: common/mem.h
===
RCS file: /cvs/src/usr.bin/vi/common/mem.h,v
retrieving revision 1.7
diff -u -p -r1.7 mem.h
--- common/mem.h7 Dec 2015 20:39:19 - 1.7
+++ common/mem.h7 Dec 2015 20:41:43 -
@@ -120,9 +120,6 @@
if (((p) = calloc((nmemb), (size))) == NULL)\
goto alloc_err; \
}
-#defineCALLOC_NOMSG(sp, p, nmemb, size) {
\
- (p) = calloc((nmemb), (size)); \
-}
#defineCALLOC_RET(sp, p, nmemb, size) {
\
if (((p) = calloc((nmemb), (size))) == NULL) { \
msgq((sp), M_SYSERR, NULL); \
@@ -137,9 +134,6 @@
#defineMALLOC_GOTO(sp, p, size) {
\
if (((p) = malloc(size)) == NULL) \
goto alloc_err; \
-}
-#defineMALLOC_NOMSG(sp, p, size) {
\
- (p) = malloc(size); \
}
#defineMALLOC_RET(sp, p, size) {
\
if (((p) = malloc(size)) == NULL) { \
[patch] fix urtw on big-endian CPU (PPC)
Hi, I'm bringing my old Mac Mini G4 back to life, and I had an issue with a wireless dongle (RTL8187 rev 0x04, RFv2). urtw interface was available, but wasn't working at all. Investigations highlighted an endianness related issue. I've written a quick and dirty patch which make the interface fully working again. I've tested it on PPC and i386 (5.8 and snapshot). Cédric --- if_urtw.c.orig 2015-11-25 04:10:00.0 +0100 +++ if_urtw.c 2015-12-07 11:40:05.0 +0100 @@ -1078,6 +1078,7 @@ USETW(req.wIndex, index); USETW(req.wLength, sizeof(uint16_t)); + data = htole16(data); return (usbd_do_request(sc->sc_udev, &req, &data)); } @@ -1587,6 +1588,7 @@ USETW(req.wLength, sizeof(uint16_t)); error = usbd_do_request(sc->sc_udev, &req, data); + *data = letoh16(*data); return (error); } @@ -1603,6 +1605,7 @@ USETW(req.wLength, sizeof(uint32_t)); error = usbd_do_request(sc->sc_udev, &req, data); + *data = letoh32(*data); return (error); } @@ -1645,6 +1648,7 @@ USETW(req.wIndex, idx & 0x03); USETW(req.wLength, sizeof(uint16_t)); + data = htole16(data); return (usbd_do_request(sc->sc_udev, &req, &data)); } @@ -1659,6 +1663,7 @@ USETW(req.wIndex, idx & 0x03); USETW(req.wLength, sizeof(uint32_t)); + data = htole32(data); return (usbd_do_request(sc->sc_udev, &req, &data)); }
Re: Remove vi allocation casting
On Mon, Dec 07, 2015 at 02:59:52PM -0500, Michael McConville wrote: > It's definitely time for these to go. > > The allocation macros would probably be better as functions (e.g. > xmalloc) these days, too. I'll save that diff for another time, though. > > No binary change. confirmed and complete diff checked by hand in the last half hour. surely producing these diffs is much more fun than reviewing them... > ok? > ok tb@
Re: [patch] Convert modulus to arc4random_uniform
On Mon, Dec 07, 2015 at 09:33:47AM +0100, Theo Buehler wrote:
> I think some of these are ok, but I'm unsure about some of the others.
> Here are some of my concerns:
>
> - since arc4random_uniform can potentially loop indefinitely, it
> might interfere with predictable timing of some routines. I can't
> tell if this is harmless in all cases below. This applies in
> particular to the proposed changes in the kernel.
I hadn't considered timing problems. I'll look at it again tonight, but
someone more familiar with the code should certainly look at it before
committing.
> - changing random() to arc4random() in games might have undesired
> side-effects like interfering with the reproducibility of tests or
> games. I think this might apply to awk for the same reason as in the
> shells.
The patch for awk was wrong. Updated patch below. I'll look into hack
tonight when I have more time.
> > Index: lib/libc/stdlib/rand.c
> > ===
> > RCS file: /cvs/src/lib/libc/stdlib/rand.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 rand.c
> > --- lib/libc/stdlib/rand.c 13 Sep 2015 08:31:47 - 1.15
> > +++ lib/libc/stdlib/rand.c 7 Dec 2015 06:42:17 -
> > @@ -50,7 +50,7 @@ int
> > rand(void)
> > {
> > if (rand_deterministic == 0)
> > - return (arc4random() % ((u_int)RAND_MAX + 1));
> > + return (arc4random_uniform((u_int)RAND_MAX + 1));
> > return (rand_r(&next));
> > }
> >
>
> this is modulo 2^n, so I think this one is fine as it is.
It's safe but takes a bit of thinking. I first had it as
return (arc4random() & RAND_MAX);
which to me is more obviously correct, but since it's safe as is. I have
no strong opinion on this.
> > Index: usr.bin/awk/run.c
> > ===
>
> Unsure about this one. I think deterministic sequences might be desired
> in some circumstances (this one is deterministic when a seed was given).
theo@ also pointed out that awk can be deterministic. Since RAND_MAX is
1 below a power of 2, & is safe. How about
Index: run.c
===
RCS file: /cvs/src/usr.bin/awk/run.c,v
retrieving revision 1.39
diff -u -p -r1.39 run.c
--- run.c 5 Sep 2015 22:07:10 - 1.39
+++ run.c 7 Dec 2015 19:28:31 -
@@ -1581,7 +1581,7 @@ Cell *bltin(Node **a, int n) /* builtin
u = (Awkfloat) system(getsval(x)) / 256; /* 256 is unix-dep */
break;
case FRAND:
- u = (Awkfloat) (random() % RAND_MAX) / RAND_MAX;
+ u = (Awkfloat) (random() & RAND_MAX) / ((u_int)RAND_MAX + 1);
break;
case FSRAND:
if (isrec(x)) { /* no argument provided */
Re: [patch] dvmrpd: strings header cleanup
On Mon, Dec 07, 2015 at 02:04:18PM -0500, Michael McConville wrote:
> Serguey Parkhomovsky wrote:
> > Fixes implicit memcpy declarations by using string.h instead of
> > strings.h, and removes strings.h from files that don't need it. Also,
> > change bzero(3) to memset(3).
>
> Thanks for this.
>
> I just took care of the existing implicit declarations. Below is a diff
> for the rest. I think you might have missed a couple bzero's.
>
> I'll look into the include removals soon. In a rush at the moment.
>
> ok?
>
OK claudio@
>
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/control.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 control.c
> --- control.c 5 Dec 2015 13:11:00 - 1.21
> +++ control.c 7 Dec 2015 19:03:15 -
> @@ -52,7 +52,7 @@ control_init(void)
> return (-1);
> }
>
> - bzero(&sun, sizeof(sun));
> + memset(&sun, 0, sizeof(sun));
> sun.sun_family = AF_UNIX;
> strlcpy(sun.sun_path, DVMRPD_SOCKET, sizeof(sun.sun_path));
>
> Index: igmp.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/igmp.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 igmp.c
> --- igmp.c18 Nov 2014 20:54:28 - 1.3
> +++ igmp.c7 Dec 2015 19:03:15 -
> @@ -51,7 +51,7 @@ send_igmp_query(struct iface *iface, str
> fatal("send_igmp_query");
>
> /* IGMP header */
> - bzero(&igmp_hdr, sizeof(igmp_hdr));
> + memset(&igmp_hdr, 0, sizeof(igmp_hdr));
> igmp_hdr.type = PKT_TYPE_MEMBER_QUERY;
>
> if (group == NULL) {
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/kroute.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 kroute.c
> --- kroute.c 27 Sep 2015 17:29:46 - 1.12
> +++ kroute.c 7 Dec 2015 19:03:15 -
> @@ -136,7 +136,7 @@ kif_find(int ifindex)
> {
> struct kif_node s;
>
> - bzero(&s, sizeof(s));
> + memset(&s, 0, sizeof(s));
> s.k.ifindex = ifindex;
>
> return (RB_FIND(kif_tree, &kit, &s));
> Index: packet.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/packet.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 packet.c
> --- packet.c 7 Dec 2015 18:59:31 - 1.4
> +++ packet.c 7 Dec 2015 19:03:15 -
> @@ -48,7 +48,7 @@ gen_dvmrp_hdr(struct ibuf *buf, struct i
> {
> struct dvmrp_hdrdvmrp_hdr;
>
> - bzero(&dvmrp_hdr, sizeof(dvmrp_hdr));
> + memset(&dvmrp_hdr, 0, sizeof(dvmrp_hdr));
> dvmrp_hdr.type = PKT_TYPE_DVMRP;
> dvmrp_hdr.code = code;
> dvmrp_hdr.chksum = 0; /* updated later */
> Index: prune.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/prune.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 prune.c
> --- prune.c 5 May 2015 01:26:37 - 1.5
> +++ prune.c 7 Dec 2015 19:03:15 -
> @@ -23,7 +23,7 @@
> #include
>
> #include
> -#include
> +#include
>
> #include "igmp.h"
> #include "dvmrpd.h"
> @@ -47,7 +47,7 @@ send_prune(struct nbr *nbr, struct prune
> if (nbr->iface->passive)
> return (0);
>
> - bzero(&prune, sizeof(prune));
> + memset(&prune, 0, sizeof(prune));
>
> dst.sin_family = AF_INET;
> dst.sin_len = sizeof(struct sockaddr_in);
> @@ -97,7 +97,7 @@ recv_prune(struct nbr *nbr, char *buf, u
> return;
> }
>
> - bzero(&p, sizeof(p));
> + memset(&p, 0, sizeof(p));
>
> prune = (struct prune_hdr *)buf;
>
> Index: rde_mfc.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/rde_mfc.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 rde_mfc.c
> --- rde_mfc.c 6 Apr 2011 11:36:26 - 1.9
> +++ rde_mfc.c 7 Dec 2015 19:03:15 -
> @@ -251,7 +251,7 @@ mfc_send_prune(struct rt_node *rn, struc
> {
> struct prunep;
>
> - bzero(&p, sizeof(p));
> + memset(&p, 0, sizeof(p));
>
> p.origin.s_addr = (mn->origin.s_addr &
> htonl(prefixlen2mask(rn->prefixlen)));
>
--
:wq Claudio
Remove vi allocation casting
It's definitely time for these to go.
The allocation macros would probably be better as functions (e.g.
xmalloc) these days, too. I'll save that diff for another time, though.
No binary change.
ok?
Index: cl/cl_main.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.26
diff -u -p -r1.26 cl_main.c
--- cl/cl_main.c29 Mar 2015 01:04:23 - 1.26
+++ cl/cl_main.c7 Dec 2015 19:49:11 -
@@ -151,7 +151,7 @@ gs_init(char *name)
name = p + 1;
/* Allocate the global structure. */
- CALLOC_NOMSG(NULL, gp, GS *, 1, sizeof(GS));
+ CALLOC_NOMSG(NULL, gp, 1, sizeof(GS));
if (gp == NULL)
perr(name, NULL);
@@ -171,7 +171,7 @@ cl_init(GS *gp)
int fd;
/* Allocate the CL private structure. */
- CALLOC_NOMSG(NULL, clp, CL_PRIVATE *, 1, sizeof(CL_PRIVATE));
+ CALLOC_NOMSG(NULL, clp, 1, sizeof(CL_PRIVATE));
if (clp == NULL)
perr(gp->progname, NULL);
gp->cl_private = clp;
Index: cl/cl_screen.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_screen.c,v
retrieving revision 1.23
diff -u -p -r1.23 cl_screen.c
--- cl/cl_screen.c 10 Apr 2015 18:05:51 - 1.23
+++ cl/cl_screen.c 7 Dec 2015 19:49:11 -
@@ -502,7 +502,7 @@ cl_getcap(SCR *sp, char *name, char **el
if ((t = tigetstr(name)) != NULL &&
t != (char *)-1 && (len = strlen(t)) != 0) {
- MALLOC_RET(sp, *elementp, char *, len + 1);
+ MALLOC_RET(sp, *elementp, len + 1);
memmove(*elementp, t, len + 1);
}
return (0);
Index: common/cut.c
===
RCS file: /cvs/src/usr.bin/vi/common/cut.c,v
retrieving revision 1.13
diff -u -p -r1.13 cut.c
--- common/cut.c12 Nov 2014 04:28:41 - 1.13
+++ common/cut.c7 Dec 2015 19:49:11 -
@@ -119,7 +119,7 @@ copyloop:
* Otherwise, if it's not an append, free its current contents.
*/
if (cbp == NULL) {
- CALLOC_RET(sp, cbp, CB *, 1, sizeof(CB));
+ CALLOC_RET(sp, cbp, 1, sizeof(CB));
cbp->name = name;
TAILQ_INIT(&cbp->textq);
LIST_INSERT_HEAD(&sp->gp->cutq, cbp, q);
@@ -300,12 +300,12 @@ text_init(SCR *sp, const char *p, size_t
{
TEXT *tp;
- CALLOC(sp, tp, TEXT *, 1, sizeof(TEXT));
+ CALLOC(sp, tp, 1, sizeof(TEXT));
if (tp == NULL)
return (NULL);
/* ANSI C doesn't define a call to malloc(3) for 0 bytes. */
if ((tp->lb_len = total_len) != 0) {
- MALLOC(sp, tp->lb, CHAR_T *, tp->lb_len);
+ MALLOC(sp, tp->lb, tp->lb_len);
if (tp->lb == NULL) {
free(tp);
return (NULL);
Index: common/exf.c
===
RCS file: /cvs/src/usr.bin/vi/common/exf.c,v
retrieving revision 1.38
diff -u -p -r1.38 exf.c
--- common/exf.c19 Nov 2015 07:53:31 - 1.38
+++ common/exf.c7 Dec 2015 19:49:11 -
@@ -86,7 +86,7 @@ file_add(SCR *sp, CHAR_T *name)
}
/* Allocate and initialize the FREF structure. */
- CALLOC(sp, frp, FREF *, 1, sizeof(FREF));
+ CALLOC(sp, frp, 1, sizeof(FREF));
if (frp == NULL)
return (NULL);
@@ -153,7 +153,7 @@ file_init(SCR *sp, FREF *frp, char *rcv_
* Default recover mail file fd to -1.
* Set initial EXF flag bits.
*/
- CALLOC_RET(sp, ep, EXF *, 1, sizeof(EXF));
+ CALLOC_RET(sp, ep, 1, sizeof(EXF));
ep->c_lno = ep->c_nlines = OOBLNO;
ep->rcv_fd = ep->fcntl_fd = -1;
F_SET(ep, F_FIRSTMODIFY);
@@ -495,7 +495,7 @@ file_spath(SCR *sp, FREF *frp, struct st
/* If we found it, build a new pathname and discard the old one. */
if (found) {
- MALLOC_RET(sp, p, char *, len + 1);
+ MALLOC_RET(sp, p, len + 1);
memcpy(p, path, len + 1);
free(frp->name);
frp->name = p;
Index: common/main.c
===
RCS file: /cvs/src/usr.bin/vi/common/main.c,v
retrieving revision 1.30
diff -u -p -r1.30 main.c
--- common/main.c 20 Nov 2015 04:12:19 - 1.30
+++ common/main.c 7 Dec 2015 19:49:11 -
@@ -360,7 +360,7 @@ editor(GS *gp, int argc, char *argv[])
size_t l;
/* Cheat -- we know we have an extra argv slot. */
l = strlen(sp->frp->name) + 1;
- MALLOC_NOMSG(sp, *--argv, char *, l);
+ MALLOC_NOMSG(sp, *--argv, l);
if (*argv == NULL) {
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On Sun, December 6, 2015 22:10, Ian Darwin wrote: > > > On 2015-12-06 12:23 PM, Stuart Henderson wrote: >> On 2015/12/06 06:02, Mickael Torres wrote: >>> Hello, >>> >>> This is a kernel patch plus a utility called ugenctl I use to allow >>> selected USB devices to attach as ugen(4) instead of their more specific >>> driver. My use case is a Microchip "PICkit 2 Microcontroller Programmer" >>> that attaches as uhid(4), but the command line utility pk2cmd wants a >>> udev(4) device. There maybe are some other use cases. >> If a device is generally pointless with uhid, we can just knock it >> out completely in the kernel, see the UQ_BAD_HID mechanism. I think this >> applies to your device. >> >> The bigger problem is "dual use" devices; e.g. some want UPS to attach >> to upd(4), others want ugen(4) for use with NUT/apcupsd. Your code is >> partially useful for these, but because it just changes things at attach, >> won't survive a reboot - if it could instead force a device to detach >> and reattach to ugen it would help a lot more cases. >> > One fairly common (I believe) use is with USB printers that attach as ulpt > but CUPS wants as ugen - for these, this is useful as it stands - just > run it > from rc. Do people want their UPS to sometimes be upd and other times be > a ugen? I can see there might be "I want to change this now" cases, but I > suspect the majority could be done once at each boot and be quite useful. > > I guess no. I have to configure kernel after every update to make my ups attach as ugen and not upd. So this tool would be useful for me with a config file so I can configure it once and don't bother on every reboot/update.
Re: [patch] dvmrpd: strings header cleanup
Serguey Parkhomovsky wrote:
> Fixes implicit memcpy declarations by using string.h instead of
> strings.h, and removes strings.h from files that don't need it. Also,
> change bzero(3) to memset(3).
This is all committed now. Thanks!
> Index: ask_nbrs2.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/ask_nbrs2.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 ask_nbrs2.c
> --- ask_nbrs2.c 5 May 2015 01:26:37 - 1.4
> +++ ask_nbrs2.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
> #include
>
> #include
> -#include
>
> #include "igmp.h"
> #include "dvmrpd.h"
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/control.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 control.c
> --- control.c 5 Dec 2015 13:11:00 - 1.21
> +++ control.c 7 Dec 2015 18:20:06 -
> @@ -52,7 +52,7 @@ control_init(void)
> return (-1);
> }
>
> - bzero(&sun, sizeof(sun));
> + memset(&sun, 0, sizeof(sun));
> sun.sun_family = AF_UNIX;
> strlcpy(sun.sun_path, DVMRPD_SOCKET, sizeof(sun.sun_path));
>
> Index: graft.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/graft.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 graft.c
> --- graft.c 5 May 2015 01:26:37 - 1.4
> +++ graft.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
> #include
>
> #include
> -#include
>
> #include "igmp.h"
> #include "dvmrpd.h"
> Index: graft_ack.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/graft_ack.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 graft_ack.c
> --- graft_ack.c 5 May 2015 01:26:37 - 1.4
> +++ graft_ack.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
> #include
>
> #include
> -#include
>
> #include "igmp.h"
> #include "dvmrpd.h"
> Index: igmp.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/igmp.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 igmp.c
> --- igmp.c 18 Nov 2014 20:54:28 - 1.3
> +++ igmp.c 7 Dec 2015 18:20:06 -
> @@ -51,7 +51,7 @@ send_igmp_query(struct iface *iface, str
> fatal("send_igmp_query");
>
> /* IGMP header */
> - bzero(&igmp_hdr, sizeof(igmp_hdr));
> + memset(&igmp_hdr, 0, sizeof(igmp_hdr));
> igmp_hdr.type = PKT_TYPE_MEMBER_QUERY;
>
> if (group == NULL) {
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/kroute.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 kroute.c
> --- kroute.c27 Sep 2015 17:29:46 - 1.12
> +++ kroute.c7 Dec 2015 18:20:06 -
> @@ -136,7 +136,7 @@ kif_find(int ifindex)
> {
> struct kif_node s;
>
> - bzero(&s, sizeof(s));
> + memset(&s, 0, sizeof(s));
> s.k.ifindex = ifindex;
>
> return (RB_FIND(kif_tree, &kit, &s));
> Index: nbrs2.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/nbrs2.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 nbrs2.c
> --- nbrs2.c 5 May 2015 01:26:37 - 1.4
> +++ nbrs2.c 7 Dec 2015 18:20:06 -
> @@ -23,7 +23,6 @@
> #include
>
> #include
> -#include
>
> #include "igmp.h"
> #include "dvmrpd.h"
> Index: packet.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/packet.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 packet.c
> --- packet.c25 Oct 2014 03:23:49 - 1.3
> +++ packet.c7 Dec 2015 18:20:07 -
> @@ -28,7 +28,7 @@
> #include
> #include
> #include
> -#include
> +#include
>
> #include "igmp.h"
> #include "dvmrpd.h"
> @@ -48,7 +48,7 @@ gen_dvmrp_hdr(struct ibuf *buf, struct i
> {
> struct dvmrp_hdrdvmrp_hdr;
>
> - bzero(&dvmrp_hdr, sizeof(dvmrp_hdr));
> + memset(&dvmrp_hdr, 0, sizeof(dvmrp_hdr));
> dvmrp_hdr.type = PKT_TYPE_DVMRP;
> dvmrp_hdr.code = code;
> dvmrp_hdr.chksum = 0; /* updated later */
> Index: prune.c
> ===
> RCS file: /cvs/src/usr.sbin/dvmrpd/prune.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 prune.c
> --- prune.c 5 May 2015 01:26:37 - 1.5
> +++ prune.c 7 Dec 2015 18:20:07 -
> @@ -23,7 +23,7 @@
> #include
>
> #include
> -#include
> +#include
>
> #include "igmp.h"
> #include "dvmrpd.h"
> @@ -47,7 +47,7 @@ send_prune(struct nbr *nbr, struct prune
> if (nbr->iface->passive)
> return (0);
>
> - bzero(&prune, sizeof(prune));
> + memset(&prune, 0, sizeof(prune));
>
> dst.sin_family = AF_INET;
> dst.sin_len = sizeof(struct sockaddr_in);
> @@ -97,7 +97,7 @@ recv_prune(struct nbr *nbr, char *buf, u
> return;
> }
>
> - bzero(&p, sizeof(p));
> + memset(&p, 0, sizeof(p));
>
> prune = (st
Re: [patch] dvmrpd: strings header cleanup
Serguey Parkhomovsky wrote:
> Fixes implicit memcpy declarations by using string.h instead of
> strings.h, and removes strings.h from files that don't need it. Also,
> change bzero(3) to memset(3).
Thanks for this.
I just took care of the existing implicit declarations. Below is a diff
for the rest. I think you might have missed a couple bzero's.
I'll look into the include removals soon. In a rush at the moment.
ok?
Index: control.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/control.c,v
retrieving revision 1.21
diff -u -p -r1.21 control.c
--- control.c 5 Dec 2015 13:11:00 - 1.21
+++ control.c 7 Dec 2015 19:03:15 -
@@ -52,7 +52,7 @@ control_init(void)
return (-1);
}
- bzero(&sun, sizeof(sun));
+ memset(&sun, 0, sizeof(sun));
sun.sun_family = AF_UNIX;
strlcpy(sun.sun_path, DVMRPD_SOCKET, sizeof(sun.sun_path));
Index: igmp.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/igmp.c,v
retrieving revision 1.3
diff -u -p -r1.3 igmp.c
--- igmp.c 18 Nov 2014 20:54:28 - 1.3
+++ igmp.c 7 Dec 2015 19:03:15 -
@@ -51,7 +51,7 @@ send_igmp_query(struct iface *iface, str
fatal("send_igmp_query");
/* IGMP header */
- bzero(&igmp_hdr, sizeof(igmp_hdr));
+ memset(&igmp_hdr, 0, sizeof(igmp_hdr));
igmp_hdr.type = PKT_TYPE_MEMBER_QUERY;
if (group == NULL) {
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/kroute.c,v
retrieving revision 1.12
diff -u -p -r1.12 kroute.c
--- kroute.c27 Sep 2015 17:29:46 - 1.12
+++ kroute.c7 Dec 2015 19:03:15 -
@@ -136,7 +136,7 @@ kif_find(int ifindex)
{
struct kif_node s;
- bzero(&s, sizeof(s));
+ memset(&s, 0, sizeof(s));
s.k.ifindex = ifindex;
return (RB_FIND(kif_tree, &kit, &s));
Index: packet.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/packet.c,v
retrieving revision 1.4
diff -u -p -r1.4 packet.c
--- packet.c7 Dec 2015 18:59:31 - 1.4
+++ packet.c7 Dec 2015 19:03:15 -
@@ -48,7 +48,7 @@ gen_dvmrp_hdr(struct ibuf *buf, struct i
{
struct dvmrp_hdrdvmrp_hdr;
- bzero(&dvmrp_hdr, sizeof(dvmrp_hdr));
+ memset(&dvmrp_hdr, 0, sizeof(dvmrp_hdr));
dvmrp_hdr.type = PKT_TYPE_DVMRP;
dvmrp_hdr.code = code;
dvmrp_hdr.chksum = 0; /* updated later */
Index: prune.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/prune.c,v
retrieving revision 1.5
diff -u -p -r1.5 prune.c
--- prune.c 5 May 2015 01:26:37 - 1.5
+++ prune.c 7 Dec 2015 19:03:15 -
@@ -23,7 +23,7 @@
#include
#include
-#include
+#include
#include "igmp.h"
#include "dvmrpd.h"
@@ -47,7 +47,7 @@ send_prune(struct nbr *nbr, struct prune
if (nbr->iface->passive)
return (0);
- bzero(&prune, sizeof(prune));
+ memset(&prune, 0, sizeof(prune));
dst.sin_family = AF_INET;
dst.sin_len = sizeof(struct sockaddr_in);
@@ -97,7 +97,7 @@ recv_prune(struct nbr *nbr, char *buf, u
return;
}
- bzero(&p, sizeof(p));
+ memset(&p, 0, sizeof(p));
prune = (struct prune_hdr *)buf;
Index: rde_mfc.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/rde_mfc.c,v
retrieving revision 1.9
diff -u -p -r1.9 rde_mfc.c
--- rde_mfc.c 6 Apr 2011 11:36:26 - 1.9
+++ rde_mfc.c 7 Dec 2015 19:03:15 -
@@ -251,7 +251,7 @@ mfc_send_prune(struct rt_node *rn, struc
{
struct prunep;
- bzero(&p, sizeof(p));
+ memset(&p, 0, sizeof(p));
p.origin.s_addr = (mn->origin.s_addr &
htonl(prefixlen2mask(rn->prefixlen)));
[patch] dvmrpd: strings header cleanup
Fixes implicit memcpy declarations by using string.h instead of
strings.h, and removes strings.h from files that don't need it. Also,
change bzero(3) to memset(3).
Index: ask_nbrs2.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/ask_nbrs2.c,v
retrieving revision 1.4
diff -u -p -r1.4 ask_nbrs2.c
--- ask_nbrs2.c 5 May 2015 01:26:37 - 1.4
+++ ask_nbrs2.c 7 Dec 2015 18:20:06 -
@@ -23,7 +23,6 @@
#include
#include
-#include
#include "igmp.h"
#include "dvmrpd.h"
Index: control.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/control.c,v
retrieving revision 1.21
diff -u -p -r1.21 control.c
--- control.c 5 Dec 2015 13:11:00 - 1.21
+++ control.c 7 Dec 2015 18:20:06 -
@@ -52,7 +52,7 @@ control_init(void)
return (-1);
}
- bzero(&sun, sizeof(sun));
+ memset(&sun, 0, sizeof(sun));
sun.sun_family = AF_UNIX;
strlcpy(sun.sun_path, DVMRPD_SOCKET, sizeof(sun.sun_path));
Index: graft.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/graft.c,v
retrieving revision 1.4
diff -u -p -r1.4 graft.c
--- graft.c 5 May 2015 01:26:37 - 1.4
+++ graft.c 7 Dec 2015 18:20:06 -
@@ -23,7 +23,6 @@
#include
#include
-#include
#include "igmp.h"
#include "dvmrpd.h"
Index: graft_ack.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/graft_ack.c,v
retrieving revision 1.4
diff -u -p -r1.4 graft_ack.c
--- graft_ack.c 5 May 2015 01:26:37 - 1.4
+++ graft_ack.c 7 Dec 2015 18:20:06 -
@@ -23,7 +23,6 @@
#include
#include
-#include
#include "igmp.h"
#include "dvmrpd.h"
Index: igmp.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/igmp.c,v
retrieving revision 1.3
diff -u -p -r1.3 igmp.c
--- igmp.c 18 Nov 2014 20:54:28 - 1.3
+++ igmp.c 7 Dec 2015 18:20:06 -
@@ -51,7 +51,7 @@ send_igmp_query(struct iface *iface, str
fatal("send_igmp_query");
/* IGMP header */
- bzero(&igmp_hdr, sizeof(igmp_hdr));
+ memset(&igmp_hdr, 0, sizeof(igmp_hdr));
igmp_hdr.type = PKT_TYPE_MEMBER_QUERY;
if (group == NULL) {
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/kroute.c,v
retrieving revision 1.12
diff -u -p -r1.12 kroute.c
--- kroute.c27 Sep 2015 17:29:46 - 1.12
+++ kroute.c7 Dec 2015 18:20:06 -
@@ -136,7 +136,7 @@ kif_find(int ifindex)
{
struct kif_node s;
- bzero(&s, sizeof(s));
+ memset(&s, 0, sizeof(s));
s.k.ifindex = ifindex;
return (RB_FIND(kif_tree, &kit, &s));
Index: nbrs2.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/nbrs2.c,v
retrieving revision 1.4
diff -u -p -r1.4 nbrs2.c
--- nbrs2.c 5 May 2015 01:26:37 - 1.4
+++ nbrs2.c 7 Dec 2015 18:20:06 -
@@ -23,7 +23,6 @@
#include
#include
-#include
#include "igmp.h"
#include "dvmrpd.h"
Index: packet.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/packet.c,v
retrieving revision 1.3
diff -u -p -r1.3 packet.c
--- packet.c25 Oct 2014 03:23:49 - 1.3
+++ packet.c7 Dec 2015 18:20:07 -
@@ -28,7 +28,7 @@
#include
#include
#include
-#include
+#include
#include "igmp.h"
#include "dvmrpd.h"
@@ -48,7 +48,7 @@ gen_dvmrp_hdr(struct ibuf *buf, struct i
{
struct dvmrp_hdrdvmrp_hdr;
- bzero(&dvmrp_hdr, sizeof(dvmrp_hdr));
+ memset(&dvmrp_hdr, 0, sizeof(dvmrp_hdr));
dvmrp_hdr.type = PKT_TYPE_DVMRP;
dvmrp_hdr.code = code;
dvmrp_hdr.chksum = 0; /* updated later */
Index: prune.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/prune.c,v
retrieving revision 1.5
diff -u -p -r1.5 prune.c
--- prune.c 5 May 2015 01:26:37 - 1.5
+++ prune.c 7 Dec 2015 18:20:07 -
@@ -23,7 +23,7 @@
#include
#include
-#include
+#include
#include "igmp.h"
#include "dvmrpd.h"
@@ -47,7 +47,7 @@ send_prune(struct nbr *nbr, struct prune
if (nbr->iface->passive)
return (0);
- bzero(&prune, sizeof(prune));
+ memset(&prune, 0, sizeof(prune));
dst.sin_family = AF_INET;
dst.sin_len = sizeof(struct sockaddr_in);
@@ -97,7 +97,7 @@ recv_prune(struct nbr *nbr, char *buf, u
return;
}
- bzero(&p, sizeof(p));
+ memset(&p, 0, sizeof(p));
prune = (struct prune_hdr *)buf;
Index: rde_mfc.c
===
RCS file: /cvs/src/usr.sbin/dvmrpd/rde_mfc.c,v
retrieving revision 1.9
diff -u -p -r1.9 rde_mfc.c
--- rde_mfc.c 6 Apr 2011 11:36:26 - 1.9
+++ rde_mfc.c 7 Dec 2015 18:20:07 -
@@ -251,7 +251,7 @@ mfc_send_prune(struct rt_node *rn, struc
{
struct prunep;
- bzero(&p, si
Re: dhclient hang
On Mon, Dec 07, 2015 at 06:36:06PM +0100, Martin Pieuchot wrote: > On 07/12/15(Mon) 18:43, Reyk Floeter wrote: > > On Mon, Dec 07, 2015 at 06:15:43PM +0100, Martin Pieuchot wrote: > > > On 07/12/15(Mon) 18:17, Reyk Floeter wrote: > > > > On Mon, Dec 07, 2015 at 04:56:56PM +0100, Martin Pieuchot wrote: > > > > > If for some reason SIOCAIFADDR fails, exit instead of hanging. > > > > > > > > > > Without this diff I need to kill dhclient(8) with ctrl+C during boot > > > > > if I happen to have the address offered by the server configured on > > > > > a different interface. > > > > > > > > > > WARNING: do not try to do that on -current without my rt_delete diffs > > > > > or your kernel with panic! > > > > > > > > > > Ok? > > > > > > > > > > > > > Why don't you reject the address, go into background and try again > > > > later? > > > > > > Because if SIOCAIFADDR fails it means that you're doing something > > > stupid so there's no point in retrying later. > > > > So what does it mean? You have two dhcp interfaces connected to the > > same VLAN? Are you doing something stupid on the local machine, the > > dhcp server, or the outside network? > > The local machine. SIOCAIFADDR failing means you're trying to configure > the same address twice. > > It's like doing: > > # ifconfig em0 192.168.0.1 > # ifconfig em1 192.168.0.1 > > The second ifconfig bails with EEXIST, just like dhclient in the case I > describe. > Yes, but if I understand the code correctly, this is called via privsep by add_address() for binding a lease that was received from the network. But, fine, a DHCP server that gives me the same IP for different MAC addresses is severely broken. I still it is wrong to abort here. Reyk > > You wouldn't believe me how much stupidity is found in data centers > > where you run your otherwise perfectly configured OpenBSD machines. > > For example, primary switch fails, backup takes over, but with a > > broken configuration ... Ah, and please don't forget about SDN where > > everything is dynamic and talks to some Linux-driven, Java- or Python > > based controllers. > > > > I don't think that dhclient should every hard fail for network- > > related errors. > > I agree, but here we're not talking about network related error. --
Re: dhclient hang
On 07/12/15(Mon) 18:43, Reyk Floeter wrote: > On Mon, Dec 07, 2015 at 06:15:43PM +0100, Martin Pieuchot wrote: > > On 07/12/15(Mon) 18:17, Reyk Floeter wrote: > > > On Mon, Dec 07, 2015 at 04:56:56PM +0100, Martin Pieuchot wrote: > > > > If for some reason SIOCAIFADDR fails, exit instead of hanging. > > > > > > > > Without this diff I need to kill dhclient(8) with ctrl+C during boot > > > > if I happen to have the address offered by the server configured on > > > > a different interface. > > > > > > > > WARNING: do not try to do that on -current without my rt_delete diffs > > > > or your kernel with panic! > > > > > > > > Ok? > > > > > > > > > > Why don't you reject the address, go into background and try again > > > later? > > > > Because if SIOCAIFADDR fails it means that you're doing something > > stupid so there's no point in retrying later. > > So what does it mean? You have two dhcp interfaces connected to the > same VLAN? Are you doing something stupid on the local machine, the > dhcp server, or the outside network? The local machine. SIOCAIFADDR failing means you're trying to configure the same address twice. It's like doing: # ifconfig em0 192.168.0.1 # ifconfig em1 192.168.0.1 The second ifconfig bails with EEXIST, just like dhclient in the case I describe. > You wouldn't believe me how much stupidity is found in data centers > where you run your otherwise perfectly configured OpenBSD machines. > For example, primary switch fails, backup takes over, but with a > broken configuration ... Ah, and please don't forget about SDN where > everything is dynamic and talks to some Linux-driven, Java- or Python > based controllers. > > I don't think that dhclient should every hard fail for network- > related errors. I agree, but here we're not talking about network related error.
Re: Make em(4) more mpsafe again
On 5.12.2015. 15:41, Claudio Jeker wrote: > So Mark and I spent some time to figure out what the issue was with ix(4) > based on that info I resurected the em(4) mpsafe diff that got backed out > and I applied the same fix. It is somewhat unclear if this fixes the > watchdog timeouts since in theory the wdog timer should be stopped when > hitting the race condition we hit in ix(4). > > I'm currently hammering my test system with this and until now I have not > seen a watchdog fired. > Hi, i have tested this patch on quad port i350 sending 1.1Mpps over routed setup and i couldn't see OACTIVE flag. doing ifconfig down/up triggers noting. this patch is combined with latest dlg@ "if_start serialisation " patch ... # netstat -i 1 em0 inem0 out total in total out packets errs packets errs colls packets errs packets errs colls 2483 0 1722 0 0 849685845 8472337 846024217 0 0 6 02 0 0 1163346 356432 1135822 0 0 3 02 0 0 1045620 474468 1044110 0 0 4 01 0 0878131 305165 878111 0 0 1 01 0 0665390 495223 632937 0 0 1 01 0 0863487 353536 835253 0 0 1 01 0 0 1032162 479482 1030798 0 0 1 01 0 0 1239190 657043 1236889 0 0 3 01 0 0803644 607252 773553 0 0 6 01 0 0 1033900 355989 1032140 0 0 3 01 0 0884158 315178 884148 0 0 3 01 0 0671153 422765 641784 0 0 2 01 0 0 1142317 302342 1141093 0 0 1 01 0 0 1227210 320112 1205720 0 0 2 02 0 0319133 423012 319073 0 0 1 01 0 0681295 480570 654932 0 0 1 01 0 0670643 490736 639768 0 0 2 01 0 0 1082213 352263 1080645 0 0 2 01 0 0678592 327973 659225 0 0 3 01 0 0898827 488901 898763 0 0 2 02 0 0711612 245223 685159 0 0 1 02 0 0685832 457282 652404 0 0 7 01 0 0 1019866 514702 994254 0 0 5 01 0 0 1167919 351723 1139686 0 0 3 01 0 0661945 477068 636373 0 0 2 01 0 0 1168973 478353 1142186 0 0 1 01 0 0320694 404637 320653 0 0 2 01 0 0667402 485570 638250 0 0 1 01 0 0665822 488057 648361 0 0 1 01 0 0673984 486514 645794 0 0 1 01 0 0676755 475929 649052 0 0 2 02 0 0673516 483569 645279 0 0 3 01 0 0676958 478505 649647 0 0 3 01 0 0671307 485305 643851 0 0 2 01 0 0678013 476246 649943 0 0 2 01 0 0673911 483511 644886 0 0 1 01 0 0674985 480910 648788 0 0 1 01 0 0673149 484188 645064 0 0 1 01 0 0676980 478390 649883 0 0 1 01 0 0672429 482019 644436 0 0 1 01 0 0677097 478082 649769 0 0 1 02 0 0676161 480272 648490 0 0 1 01 0 0671737 480768 643312 0 0 1 02 0 0838715 358990 809036 0 0 2 01 0 0 1165676 880005 1137991 0 0 2 01 0 0836085 354568 835948 0 0 1 02 0 0618887 508218 590496 0 0 1 01 0 0939546 489689 900721 0 0 1 01 0 0936974 605481 908905 0 0 1 01 0 0 1040186 355489 1038637 0 0 1 01 0 0966385 322910 939540 0 0 1 01 0 0829643 482648 829587 0 0 1 01 0 0646072 409609 617855 0 0 1 01 0 0915205 358060 915191 0 0 2 01 0 0641707 402106 618920 0 0 1 01 0 0906929 35836
Re: Overflowable int -> size_t in grep
On Mon, 07 Dec 2015 02:32:50 -0500, Michael McConville wrote: > Does this look better? Or is PRId64 preferred for off_t? As Otto said, we generally use %lld for printing off_t and use a (long long) cast. With that change OK millert@ - todd
Re: Overflowable int -> size_t in grep
> > Otto Moerbeek wrote: > > > On Mon, Dec 07, 2015 at 01:36:22AM -0500, Michael McConville wrote: > > > > This isn't a grave issue, but I came across it while exploring integer > > > > overflow and think it's worth sharing. > > > > > > > > grep represents line numbers with an int, which predictably overflows > > > > for inputs with >= 2^31 newlines. This is easy to demonstrate using the > > > > -n option and a debugging printf. > > > > > > > > The below diff fixes this, and tunes up a for loop while I'm in there. > > > > > > how does this fix work on 32-bit platforms? Better use offset_t. > > > > Good catch, thanks. > > > > Does this look better? Or is PRId64 preferred for off_t? > > Neither is correct, off_t should be cast to intmax_t first and then %jd > be used. That is incorrect advice for the OpenBSD world.
dhclient hang
If for some reason SIOCAIFADDR fails, exit instead of hanging.
Without this diff I need to kill dhclient(8) with ctrl+C during boot
if I happen to have the address offered by the server configured on
a different interface.
WARNING: do not try to do that on -current without my rt_delete diffs
or your kernel with panic!
Ok?
Index: kroute.c
===
RCS file: /cvs/src/sbin/dhclient/kroute.c,v
retrieving revision 1.75
diff -u -p -r1.75 kroute.c
--- kroute.c10 Feb 2015 04:20:26 - 1.75
+++ kroute.c7 Dec 2015 15:51:17 -
@@ -473,7 +473,7 @@ priv_add_address(struct imsg_add_address
/* No need to set broadcast address. Kernel can figure it out. */
if (ioctl(s, SIOCAIFADDR, &ifaliasreq) == -1)
- warning("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
+ error("SIOCAIFADDR failed (%s): %s", inet_ntoa(imsg->addr),
strerror(errno));
close(s);
iwm association fix
While debugging my 802.11n code I got tired of iwm randomly failing
to associate or even getting into a state where I had to reboot to
get it to work again.
In case it got hung, the newstate task was sleeping in "iwmauth" and
never woke up, which made me revisit how this driver transitions to
AUTH state. I couldn't figure out exactly how it got hung, but I
believe I found a good reason to remove this tsleep entirely.
What's going on here is that the firmware can hop channels in the
background whenever it feels like doing so. To prevent this from
happening during the association procedure, the driver schedules a
"time event" which makes the firmware stay on the current channel
for a specified amount of time. This approach is borrowed from the
Linux driver.
If all goes well, the following happens:
1) The driver's newstate task asks for the time event and goes into
tsleep() to wait for the firmware to signal the beginning of the
time event.
2) The firmware signals beginning of the time event.
3) The interrupt handler sets a flag in the softc which indicates
"time event start" and wakes up the sleeping newstate task.
4) The newstate task is scheduled, checks the flag which says that
the time event has started, and finishes moving to AUTH state.
5) Association continues and completes
6) The firmware signals the end of the time event.
7) The interrupt handler sets a flag in the softc which indicates
"time event ended" and wakes up any task sleeping on this
condition (there is none)
Note that steps 5 and 6/7 can happen "concurrently" if the "time-event
ended" interrupt triggers during step 5, which is no problem.
Sometimes, especially with IWM_DEBUG enabled (lots of printf), this
doesn't work out as planned and the order of events goes like this:
1) The driver's newstate task asks for the time event and goes into
tsleep() to wait for the firmware to signal the beginning of the
time event.
2) The firmware signals beginning of the time event.
3) The interrupt handler sets a flag in the softc which indicates
"time event start" and wakes up the sleeping newstate task.
4) The firmware signals the end of the time event.
5) The interrupt handler sets a flag in the softc which indicates
"time event ended" and wakes up the sleeping newstate task.
6) The newstate task is scheduled, checks the flag which says that
the time event has started, but it is not set so association is
aborted.
I have also observed other variants of these chains of events where
iwm does not manage to associate.
Leaving precise synchronization like this up to the scheduler in our
kernel is not a good idea. There is no guarantee that things will work
out as intended. Moreover, the numbers controlling timing were copied
from Linux which has different scheduling latency.
This diff does away with the unreliable extra checking. It schedules a
time event, busy-waits a short while for good measure, and life goes on.
The firmware might fail to schedule the event, and if so might wander off
the channel for a moment... in which case association is still more
likely to succeed than with these unreliable checks in place.
But generally the time event will be scheduled and things just work out.
While here, also copy some related docs in comments from the Linux driver
and remove an unused argument from iwm_mvm_protect_session().
OK?
Below the diff is a trace which shows how events play out now.
The lines saying "iwm0: TE notif status = 0x01 action = 0x01" indicate
time event start, and the same with "action = 0x02" indicates the end
of the time event -- well after association is complete.
Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.68
diff -u -p -r1.68 if_iwm.c
--- if_iwm.c25 Nov 2015 03:09:59 - 1.68
+++ if_iwm.c7 Dec 2015 15:13:22 -
@@ -272,7 +272,7 @@ int iwm_mvm_send_time_event_cmd(struct i
intiwm_mvm_time_event_send_add(struct iwm_softc *, struct iwm_node *,
void *, struct iwm_time_event_cmd_v2 *);
void iwm_mvm_protect_session(struct iwm_softc *, struct iwm_node *,
- uint32_t, uint32_t, uint32_t);
+ uint32_t, uint32_t);
intiwm_nvm_read_chunk(struct iwm_softc *, uint16_t, uint16_t, uint16_t,
uint8_t *, uint16_t *);
intiwm_nvm_read_section(struct iwm_softc *, uint16_t, uint8_t *,
@@ -2216,7 +2216,7 @@ iwm_mvm_time_event_send_add(struct iwm_s
void
iwm_mvm_protect_session(struct iwm_softc *sc, struct iwm_node *in,
- uint32_t duration, uint32_t min_duration, uint32_t max_delay)
+ uint32_t duration, uint32_t max_delay)
{
struct iwm_time_event_cmd_v2 time_cmd;
@@ -4988,7 +4988,6 @@ iwm_auth(struct iwm_softc *sc)
struct ieee80211com *ic = &sc->sc_ic;
struct iwm_node *in = (void *)ic->ic_bss;
uint32_t
umass: size for free
Hello,
I worked a bit on umass(4) recently and had a diff to pass the
umassbus_softc's real size to free so here it is. At some point I
pondered about deleting the whole abstraction, as it would simplify the
free'ing, for we only have one implementation (umass_scsi_softc, as atapi
uses it too). But I figured it would be against the whole design of the
umass driver, thoughts?
Index: usb/umass.c
===
RCS file: /cvs/src/sys/dev/usb/umass.c,v
retrieving revision 1.70
diff -u -p -r1.70 umass.c
--- usb/umass.c 14 Mar 2015 03:38:50 - 1.70
+++ usb/umass.c 7 Dec 2015 15:40:15 -
@@ -651,7 +651,7 @@ umass_detach(struct device *self, int fl
if (scbus != NULL) {
if (scbus->sc_child != NULL)
rv = config_detach(scbus->sc_child, flags);
- free(scbus, M_DEVBUF, 0);
+ free(scbus, M_DEVBUF, scbus->sc_size);
sc->bus = NULL;
}
Index: usb/umass_scsi.c
===
RCS file: /cvs/src/sys/dev/usb/umass_scsi.c,v
retrieving revision 1.42
diff -u -p -r1.42 umass_scsi.c
--- usb/umass_scsi.c14 Mar 2015 03:38:50 - 1.42
+++ usb/umass_scsi.c7 Dec 2015 15:40:16 -
@@ -145,6 +145,7 @@ umass_scsi_setup(struct umass_softc *sc)
struct umass_scsi_softc *scbus;
scbus = malloc(sizeof(*scbus), M_DEVBUF, M_WAITOK | M_ZERO);
+ scbus->base.sc_size = sizeof(*scbus);
sc->bus = (struct umassbus_softc *)scbus;
Index: usb/umassvar.h
===
RCS file: /cvs/src/sys/dev/usb/umassvar.h,v
retrieving revision 1.14
diff -u -p -r1.14 umassvar.h
--- usb/umassvar.h 6 Nov 2013 14:37:31 - 1.14
+++ usb/umassvar.h 7 Dec 2015 15:40:16 -
@@ -146,6 +146,7 @@ struct umass_wire_methods {
struct umassbus_softc {
struct device *sc_child; /* child device, for detach */
+ size_t sc_size;
};
/* the per device structure */
rtdeletemsg & KASSERT
The rtrequest_delete() refactoring exposed an existing bug and
introduced a regression, both triggered by the same KASSERT().
The regression has been reported there:
https://marc.info/?l=openbsd-bugs&m=144943901304713&w=2
The problem is that rt_if_remove() will triggers a rtflushclone1() if a
RTF_CLONING route is attached to an interface. In this case
rtdeletemsg() tries to get the interface pointer from rt_ifidx and the
KASSERT() fires.
The bug exposed by the same KASSERT() can be triggered by a call to
rt_ifa_del() when dhclient(8) tries to add an address already configured
on the system. In this case the kernel calls rtrequest_delete() with
a wrong ifp.
Diff below fixes these two bugs. It's big because I don't want to put
two customs checks in a generic function. This diff is built upon my
previous rtdeletemsg() one.
It introduces rt_delete() which deletes a route *without* doing a
lookup. There's various good reasons for that.
- First of all I don't want to take the risk of matching a similar
multipath route. The rt_ifa_del() bug is an example.
- Secondly I'd like to avoid a recursive rtable_* call when routes
are deleted from rtable_walk().
- Then I'd rather keep userland-specific checks inside rtrequest()
which will ends up in rtsock.c one day.
- Finally it cleans one use of "struct rt_addrinfo" which should be
limited to userland in order to get rid of RTA_NETMASK in kernel.
Note that it introduces a behavior change. If rtdeletemsg() fails to
delete a route no message is reported to userland. I believe this was
only used for debugging.
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.289
diff -u -p -r1.289 route.c
--- net/route.c 5 Dec 2015 10:07:55 - 1.289
+++ net/route.c 7 Dec 2015 15:10:15 -
@@ -159,8 +159,7 @@ struct sockaddr *rt_plentosa(sa_family_t
struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *,
u_int);
-intrtrequest_delete(struct rt_addrinfo *, u_int8_t, struct ifnet *,
- struct rtentry **, u_int);
+intrt_delete(struct rtentry *, struct ifnet *, unsigned int);
#ifdef DDB
void db_print_sa(struct sockaddr *);
@@ -613,38 +612,29 @@ out:
}
/*
- * Delete a route and generate a message
+ * Delete a route and generate a message. The caller must hold a reference
+ * on ``rt''.
*/
int
-rtdeletemsg(struct rtentry *rt, struct ifnet *ifp, u_int tableid)
+rtdeletemsg(struct rtentry *rt, struct ifnet *ifp, unsigned int rtableid)
{
int error;
- struct rt_addrinfo info;
- unsigned intifidx;
- struct sockaddr_in6 sa_mask;
- /*
-* Request the new route so that the entry is not actually
-* deleted. That will allow the information being reported to
-* be accurate (and consistent with route_output()).
-*/
- bzero((caddr_t)&info, sizeof(info));
- info.rti_info[RTAX_DST] = rt_key(rt);
- if (!ISSET(rt->rt_flags, RTF_HOST))
- info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt, &sa_mask);
- info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
- info.rti_flags = rt->rt_flags;
- ifidx = rt->rt_ifidx;
- error = rtrequest_delete(&info, rt->rt_priority, ifp, &rt, tableid);
- rt_missmsg(RTM_DELETE, &info, info.rti_flags, ifidx, error, tableid);
+ KASSERT(rt->rt_ifidx == ifp->if_index);
+
+ error = rt_delete(rt, ifp, rtableid);
if (error == 0)
- rtfree(rt);
+ rt_sendmsg(rt, RTM_DELETE, rtableid);
+
return (error);
}
static inline int
rtequal(struct rtentry *a, struct rtentry *b)
{
+ if (a == b)
+ return 1;
+
if (memcmp(rt_key(a), rt_key(b), rt_key(a)->sa_len) == 0 &&
rt_plen(a) == rt_plen(b))
return 1;
@@ -656,10 +646,25 @@ int
rtflushclone1(struct rtentry *rt, void *arg, u_int id)
{
struct rtentry *parent = arg;
+ struct ifnet *ifp;
+
+ ifp = if_get(rt->rt_ifidx);
+
+ /*
+* This happens when an interface with a RTF_CLONING route is
+* being detached. In this case it's safe to bail because all
+* the routes are being purged by rt_if_remove().
+*/
+ if (ifp == NULL)
+ return 0;
- if ((rt->rt_flags & RTF_CLONED) != 0 && (rt->rt_parent == parent ||
- rtequal(rt->rt_parent, parent)))
- rtdeletemsg(rt, NULL, id);
+ if (ISSET(rt->rt_flags, RTF_CLONED) && rtequal(rt->rt_parent, parent)) {
+ rtref(rt);
+ rtdeletemsg(rt, ifp, id);
+ rtfree(rt);
+ }
+
+ if_put(ifp);
return 0;
}
@@ -801,53 +806,20 @@ rt_getifa(struct rt_addrinfo *info, u_in
}
int
-rtrequest_delete(struct rt_addrinfo *info, u_int8_t prio, struct ifnet *ifp,
-struct rtentry **ret_n
Re: preparing multitouch support - request for tests
On Thu, Dec 03, 2015 at 12:20:15AM +0100, Ulf Brosziewski wrote: > The diffs below contain a complete and extensive rewrite of the > input-processing parts of wsmouse and the interface it provides to > the hardware drivers. It prepares the support for various kinds of > multitouch input, as well as an extended support for touchpads by > wsmouse. > Hi, it looks like the diff got some white space mangled somewhere on the path to my mailbox. Would you have a version available somewhere with a more reliable transport than email ? Thank you. -- Matthieu Herrb pgpD4R376sQfx.pgp Description: PGP signature
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On 07/12/15(Mon) 22:18, Xiaofan Chen wrote: > On Mon, Dec 7, 2015 at 7:42 PM, Martin Pieuchot wrote: > > I'd really prefer a solution that doesn't need any button. In other > > words to always be able to use your device from userland if the kernel > > is not using it. > > > > One way would be to always attach a ugen(4) driver to every USB device, > > I think that's what FreeBSD does. The other way would be to use the > > usb(4) bus interface to talk to your device. In both cases some work > > is needed to guarantee that an endpoint is only driven by one driver at > > a time. Right now this is done by attaching only one driver. > > It will be good if this limitation is removed (I know the FreeBSD approach > but FreeBSD libusb is not under libusb project due to license concerns) > and then libusb will be more useful. Or a method like Linux where the > kernel driver can be dynamically detached (so that usbfs generic driver > is attached and libusb uses usbfs under Linux). I agree, but somebody has to do the work 8) Starting with a HID device is the way to go. The first step should be to deprecate the libusbhid(3) and use the libusb instead. Userland should stop using it, then we disable it in the kernel making every HID device attach as ugen(4) and switch the ports currently using libusbhid to libusb. > > To which driver your device currently attaches? What kind of transfers > > to you want to submit from userland? The libusb already allow your to > > submit(some) control transfers. It would be nice if you could improve > > this rather than adding a hack like this one. > > > > As mentioned in another email reply, PICKit 2 is a generic USB device > and it uses control transfer and interrupt transfer. It is a bit unique that > PICkit 2 has two USB configurations, one for HID and one for custom > USB ( a legacy from the PICKit 1 days). Well having two configurations should not be a problem.
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On 2015/12/07 22:11, Xiaofan Chen wrote: > Not so sure if there is a port of hidapi under OpenBSD. > Ref: http://www.signal11.us/oss/hidapi/ It isn't ported to OpenBSD, and it's one of those projects where they don't provide any infrastructure to build a library and suggest that people copy it to their source tree and add to their own Makefiles..
Re: taskctx and revisiting if_start serialisation
> On 7 Dec 2015, at 11:34 PM, Martin Pieuchot wrote: > > On 07/12/15(Mon) 19:37, David Gwynne wrote: >> On Sun, Dec 06, 2015 at 03:51:26PM +0100, Martin Pieuchot wrote: >>> On 06/12/15(Sun) 14:00, David Gwynne wrote: [...] the idea is you have a taskctx, which represents a serialising context for tasks. tasks are submitted to the taskctx, and the code will try to run the tasks immediately rather than defer them to a thread. if there is contention on the context, the contending cpu yields after queueing the task because the other cpu is responsible for running all pending tasks to completion. it also simplifies the barrier operations a lot. the diff below implements a generic taskctx framework, and cuts the mpsafe if_start() implementation over to it. >>> >>> I'm not sure this should be implemented as a generic framework. I'd >>> suggest to keep it specific to if_start(). As you say above the least >>> worst mechanism we have is currently taskqs, but maybe this could/will >>> change? >> >> im in two minds about where the ctx code should sit. i obviously >> lean toward keeping it with taskqs, but i also cant come up with >> another use case for it. i also havent tried very hard to think of >> one. > > Even if you have another use case for it I find this generic framework > confusing. It is build on top of taskq, but it's not part of the taskq > API. So putting the documentation in the same manual just confuse me. > Your task API is simple and people use it easily. I think it should > stay like that. > > Now we need a way to serialize start and txeof and you came up with a > solution... > >> are you asking if taskqs will get better, or if something better >> than taskqs will come along? > > ...I'm just saying that the "implementation" of your serialisation > mechanism should not matter. I'm fine with having a context for tasks > but I'd suggest that this should be abstracted in a simple API instead > of offering a framework to build upon. ill tweak it in the morning. > Could you make it such that you don't need a task in myx_softc? sure. the end extreme would be having an if_txeof() function in if.c that wraps a lot of this up. however, i dont want to dictate that start and txeof have to be serialised. if you're careful it is possible to operate both the start and txeof side of a ring concurrently. the only coordination necessary is around the oactive handling. if some drivers want to completely serialise both start and txeof they should be free to do, but if they want to make it as concurrent as possible then they should be able to what myx is doing and only coordinate oactive. ill tinker with it tomorrow though. > >>> I cannot understand what's happening by reading the myx(4) chunk itself. >>> So I believe the interface needs to be polished. Would it be possible >>> to implement this serialization without introducing if_restart()? >> >> that is by far my least favourite bit of this diff. in my defense, >> i wrote it while my mother was trying to have a conversation with >> me, so it didn't get my full attention. ive torn if_restart out and >> implemented it completely in myx below. >> myx is also changed to only clr oactive from within the taskctx serialiser, thereby avoiding the race, but keeps the bulk of txeof outside the serialiser so it can run concurrently with start. other nics are free to serialise start and txeof within the ifq_serializer if they want, or not, it is up to them. thoughts? tests? opinions on messy .h files? >>> >>> It appears to me that you have unrelated changes: if_enter/if_leave. >> >> oops, yes. i suck at juggling diffs. maybe we should call if.c >> full and split it up ;) > > I think we should start with that because the I'm afraid we're already > cooking spaghetti. > > My question is the barrier and mpsafe code is related to a queue or to > an interface? In other words does it make sense to serialize on a > context in the sending queue? What about multiple queues? it's per queue. ifnet and ifqueue are mixed together atm cos we've long had a 1:1 mapping between them in our stack. right now im trying to make the code mpsafe. keeping the api change simple means passing an ifp when an ifq is technically more correctly. making it work for multiple queues is something to do properly in the future. ill try to be more careful about this now to avoid confusion though by at least keeping the data structures grouped properly. dlg
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On Mon, Dec 7, 2015 at 7:42 PM, Martin Pieuchot wrote: > I'd really prefer a solution that doesn't need any button. In other > words to always be able to use your device from userland if the kernel > is not using it. > > One way would be to always attach a ugen(4) driver to every USB device, > I think that's what FreeBSD does. The other way would be to use the > usb(4) bus interface to talk to your device. In both cases some work > is needed to guarantee that an endpoint is only driven by one driver at > a time. Right now this is done by attaching only one driver. It will be good if this limitation is removed (I know the FreeBSD approach but FreeBSD libusb is not under libusb project due to license concerns) and then libusb will be more useful. Or a method like Linux where the kernel driver can be dynamically detached (so that usbfs generic driver is attached and libusb uses usbfs under Linux). > To which driver your device currently attaches? What kind of transfers > to you want to submit from userland? The libusb already allow your to > submit(some) control transfers. It would be nice if you could improve > this rather than adding a hack like this one. > As mentioned in another email reply, PICKit 2 is a generic USB device and it uses control transfer and interrupt transfer. It is a bit unique that PICkit 2 has two USB configurations, one for HID and one for custom USB ( a legacy from the PICKit 1 days). -- Xiaofan
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On Mon, Dec 7, 2015 at 8:19 PM, Mickael Torres wrote: > > Actually the device attaches as uhid. I don't know which kind of transfer > is used, I'll have to look into this, but the soft (pk2cmd) uses libusb > and only works when the device is attached as ugen. pk2cmd is only for PICkit 2 which is an HID device for the default configuration. It actually has two configurations and the alternative configuration is a generic USB device. I have not tried it under OpenBSD and not so sure if it can use the alternative configuration. Not so sure if there is a port of hidapi under OpenBSD. Ref: http://www.signal11.us/oss/hidapi/ We at the libusb project now recommend HIDAPI for generic USB HID device like PICKit 2. https://github.com/libusb/libusb/wiki/FAQ#Does_libusb_support_HID_devices When Jeff Post worked with Microchip developer to get pk2cmd to work under Linux, there was no HIDAPI, only libusb-0.1. I was involved heavily at that time to get pk2cmd to work under Linux and I actually got pk2cmd to work under FreeBSD last time. https://github.com/psmay/pk2cmd/blob/master/pk2cmd/ReadmeMakefile.txt > And another soft (pic32prog) which also uses libusb seems to only detect > the device when it is attached as uhid. > I'll try to take a look into libusb. pic32prog seems to supports different USB interface but many of them uses HID interface. pk2cmd code github mirror (license not considered to be open source). https://github.com/psmay/pk2cmd The Linux backend for pk2cmd is this one. https://github.com/psmay/pk2cmd/blob/master/pk2cmd/pk2usb.cpp I have submitted kernel patch to Linux kernel to blacklist PICkit 2 under Linux so it is not attached to the kernel HID driver. Ref: http://mcuee.blogspot.sg/2008/08/two-trivial-patches-now-in-linux-kernel.html pic32prog seems to be here. https://github.com/sergev/pic32prog -- Xiaofan
Do not pass NULL to rtdeletemsg()
If the interface is gone that means you're dealing with a cached route
so there's no need to try to remove it from the table.
Better be explicit and do that before calling rtdeletemsg() rather than
inside.
ok?
Index: netinet/ip_icmp.c
===
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.150
diff -u -p -r1.150 ip_icmp.c
--- netinet/ip_icmp.c 3 Dec 2015 21:11:53 - 1.150
+++ netinet/ip_icmp.c 7 Dec 2015 12:40:06 -
@@ -1042,19 +1042,21 @@ icmp_mtudisc(struct icmp *icp, u_int rta
void
icmp_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp_mtudisc_timeout: bad route to timeout");
+ struct ifnet *ifp;
+ int s;
- if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_DYNAMIC | RTF_HOST)) {
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
void *(*ctlfunc)(int, struct sockaddr *, u_int, void *);
struct sockaddr_in sin;
- int s;
sin = *satosin(rt_key(rt));
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
/* Notify TCP layer of increased Path MTU estimate */
ctlfunc = inetsw[ip_protox[IPPROTO_TCP]].pr_ctlinput;
@@ -1062,9 +1064,12 @@ icmp_mtudisc_timeout(struct rtentry *rt,
(*ctlfunc)(PRC_MTUINC, sintosa(&sin),
r->rtt_tableid, NULL);
splx(s);
- } else
+ } else {
if ((rt->rt_rmx.rmx_locks & RTV_MTU) == 0)
rt->rt_rmx.rmx_mtu = 0;
+ }
+
+ if_put(ifp);
}
/*
@@ -1088,17 +1093,20 @@ icmp_ratelimit(const struct in_addr *dst
void
icmp_redirect_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp_redirect_timeout: bad route to timeout");
+ struct ifnet *ifp;
+ int s;
- if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_DYNAMIC | RTF_HOST)) {
- int s;
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
}
+
+ if_put(ifp);
}
int
Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.182
diff -u -p -r1.182 icmp6.c
--- netinet6/icmp6.c3 Dec 2015 21:11:53 - 1.182
+++ netinet6/icmp6.c7 Dec 2015 12:39:28 -
@@ -1952,34 +1952,42 @@ icmp6_mtudisc_clone(struct sockaddr *dst
void
icmp6_mtudisc_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp6_mtudisc_timeout: bad route to timeout");
- if ((rt->rt_flags & (RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_DYNAMIC | RTF_HOST)) {
- int s;
+ struct ifnet *ifp;
+ int s;
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
} else {
if (!(rt->rt_rmx.rmx_locks & RTV_MTU))
rt->rt_rmx.rmx_mtu = 0;
}
+
+ if_put(ifp);
}
void
icmp6_redirect_timeout(struct rtentry *rt, struct rttimer *r)
{
- if (rt == NULL)
- panic("icmp6_redirect_timeout: bad route to timeout");
- if ((rt->rt_flags & (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) ==
- (RTF_GATEWAY | RTF_DYNAMIC | RTF_HOST)) {
- int s;
+ struct ifnet *ifp;
+ int s;
+ ifp = if_get(rt->rt_ifidx);
+ if (ifp == NULL)
+ return;
+
+ if ((rt->rt_flags & (RTF_DYNAMIC|RTF_HOST)) == (RTF_DYNAMIC|RTF_HOST)) {
s = splsoftnet();
- rtdeletemsg(rt, NULL, r->rtt_tableid);
+ rtdeletemsg(rt, ifp, r->rtt_tableid);
splx(s);
}
+
+ if_put(ifp);
}
int *icmpv6ctl_vars[ICMPV6CTL_MAXID] = ICMPV6CTL_VARS;
Re: taskctx and revisiting if_start serialisation
On 07/12/15(Mon) 19:37, David Gwynne wrote:
> On Sun, Dec 06, 2015 at 03:51:26PM +0100, Martin Pieuchot wrote:
> > On 06/12/15(Sun) 14:00, David Gwynne wrote:
> > > [...]
> > > the idea is you have a taskctx, which represents a serialising
> > > context for tasks. tasks are submitted to the taskctx, and the code
> > > will try to run the tasks immediately rather than defer them to a
> > > thread. if there is contention on the context, the contending cpu
> > > yields after queueing the task because the other cpu is responsible
> > > for running all pending tasks to completion.
> > >
> > > it also simplifies the barrier operations a lot.
> > >
> > > the diff below implements a generic taskctx framework, and cuts the
> > > mpsafe if_start() implementation over to it.
> >
> > I'm not sure this should be implemented as a generic framework. I'd
> > suggest to keep it specific to if_start(). As you say above the least
> > worst mechanism we have is currently taskqs, but maybe this could/will
> > change?
>
> im in two minds about where the ctx code should sit. i obviously
> lean toward keeping it with taskqs, but i also cant come up with
> another use case for it. i also havent tried very hard to think of
> one.
Even if you have another use case for it I find this generic framework
confusing. It is build on top of taskq, but it's not part of the taskq
API. So putting the documentation in the same manual just confuse me.
Your task API is simple and people use it easily. I think it should
stay like that.
Now we need a way to serialize start and txeof and you came up with a
solution...
> are you asking if taskqs will get better, or if something better
> than taskqs will come along?
...I'm just saying that the "implementation" of your serialisation
mechanism should not matter. I'm fine with having a context for tasks
but I'd suggest that this should be abstracted in a simple API instead
of offering a framework to build upon.
Could you make it such that you don't need a task in myx_softc?
> > I cannot understand what's happening by reading the myx(4) chunk itself.
> > So I believe the interface needs to be polished. Would it be possible
> > to implement this serialization without introducing if_restart()?
>
> that is by far my least favourite bit of this diff. in my defense,
> i wrote it while my mother was trying to have a conversation with
> me, so it didn't get my full attention. ive torn if_restart out and
> implemented it completely in myx below.
>
> > > myx is also changed to only clr oactive from within the taskctx
> > > serialiser, thereby avoiding the race, but keeps the bulk of txeof
> > > outside the serialiser so it can run concurrently with start.
> > >
> > > other nics are free to serialise start and txeof within the
> > > ifq_serializer if they want, or not, it is up to them.
> > >
> > > thoughts? tests? opinions on messy .h files?
> >
> > It appears to me that you have unrelated changes: if_enter/if_leave.
>
> oops, yes. i suck at juggling diffs. maybe we should call if.c
> full and split it up ;)
I think we should start with that because the I'm afraid we're already
cooking spaghetti.
My question is the barrier and mpsafe code is related to a queue or to
an interface? In other words does it make sense to serialize on a
context in the sending queue? What about multiple queues?
> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.422
> diff -u -p -r1.422 if.c
> --- sys/net/if.c 5 Dec 2015 10:07:55 - 1.422
> +++ sys/net/if.c 7 Dec 2015 06:30:42 -
> @@ -153,8 +153,9 @@ void if_input_process(void *);
> void ifa_print_all(void);
> #endif
>
> -void if_start_mpsafe(struct ifnet *ifp);
> -void if_start_locked(struct ifnet *ifp);
> +void if_start_mpsafe(struct ifnet *);
> +void if_start_locked(struct ifnet *);
> +void if_start_task(void *);
>
> /*
> * interface index map
> @@ -513,6 +514,7 @@ if_attach_common(struct ifnet *ifp)
> TAILQ_INIT(&ifp->if_maddrlist);
>
> ifq_init(&ifp->if_snd);
> + task_set(&ifp->if_start_ctx, if_start_task, ifp);
>
> ifp->if_addrhooks = malloc(sizeof(*ifp->if_addrhooks),
> M_TEMP, M_WAITOK);
> @@ -557,66 +559,29 @@ if_start_locked(struct ifnet *ifp)
> KERNEL_UNLOCK();
> }
>
> -static inline unsigned int
> -ifq_enter(struct ifqueue *ifq)
> -{
> - return (atomic_inc_int_nv(&ifq->ifq_serializer) == 1);
> -}
> -
> -static inline unsigned int
> -ifq_leave(struct ifqueue *ifq)
> +void
> +if_start_mpsafe(struct ifnet *ifp)
> {
> - if (atomic_cas_uint(&ifq->ifq_serializer, 1, 0) == 1)
> - return (1);
> -
> - ifq->ifq_serializer = 1;
> -
> - return (0);
> + task_dispatch(&ifp->if_snd.ifq_serializer, &ifp->if_start_ctx);
> }
>
> void
> -if_start_mpsafe(struct ifnet *ifp)
> +if_start_task(void *p)
> {
> - struct ifqueue *ifq = &ifp->if_snd;
> +
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On 07/12/15(Mon) 13:19, Mickael Torres wrote: > [...] > Actually the device attaches as uhid. I don't know which kind of transfer > is used, I'll have to look into this, but the soft (pk2cmd) uses libusb > and only works when the device is attached as ugen. That's easy because the uhid(4) driver does nothing but offers an userland interface. > And another soft (pic32prog) which also uses libusb seems to only detect > the device when it is attached as uhid. This generally means it uses the libusbhid. Could you try to build the port using libusb instead?
Re: taskctx and revisiting if_start serialisation
On 6.12.2015. 15:56, Hrvoje Popovski wrote: > On 6.12.2015. 5:00, David Gwynne wrote: >> the current code for serialising if_start calls for mpsafe nics does what it >> says. >> >> however, kettenis realised it doesnt help us much when we're trying >> to coordinate between the start and txeof side of a driver when >> setting or clearing oactive. in particular, a start routine can >> figure out there's no more space, and then set oactive. txeof could >> be running on another cpu emptying the ring and clearing it. if >> that clear runs in between the other cpus space check and >> ifq_set_oactive, then the nic will be marked full and the stack >> wont ever call start on it again. >> >> so it can be argued that start and txeof should be serialised. >> indeed, other platforms do exactly that. >> >> the least worst mechanism we have for doing that is taskqs. however, >> all my experiments deferring start to a taskq end up significantly >> hurting performance. >> >> dragonfly appears to have some of the semantics we want. according >> to sephe, start and txeof are serialised, but they can be directly >> called from anywhere. however, if one cpu is trying to run start >> while the other is in txeof, it figures it out and makes the other >> cpu run txeof on the first cpus behalf. the first cpu then simply >> returns cos it knows the other cpu will end up doing the work. >> >> the implementation is tied very much to that specific situation, >> and its hard for me to grok cos im not familiar with their locking >> infrastructure. >> >> the dfly code has the (slight) caveat that you cant run txeof and >> start concurrently, it forces them to be serialised. >> >> while toying with ideas on how to solve kettenis' oactive problem, >> i came up with the following. >> >> it combines tasks with direct dispatch, and borrows the current >> ifq_serialiser/pool/scsi serialisation algorithm. >> >> the idea is you have a taskctx, which represents a serialising >> context for tasks. tasks are submitted to the taskctx, and the code >> will try to run the tasks immediately rather than defer them to a >> thread. if there is contention on the context, the contending cpu >> yields after queueing the task because the other cpu is responsible >> for running all pending tasks to completion. >> >> it also simplifies the barrier operations a lot. >> >> the diff below implements a generic taskctx framework, and cuts the >> mpsafe if_start() implementation over to it. >> >> myx is also changed to only clr oactive from within the taskctx >> serialiser, thereby avoiding the race, but keeps the bulk of txeof >> outside the serialiser so it can run concurrently with start. >> >> other nics are free to serialise start and txeof within the >> ifq_serializer if they want, or not, it is up to them. >> >> thoughts? tests? opinions on messy .h files? > > > Hi, > > after applying this patches over cvs source from few hours (no > additional patches for ix and em) it seems that something isn't right... > > freshly rebooted box, sending 2Mpps over ix (82599) and i'm getting > around 50kpps on receiver... over x540 around 100kpps With latest patch 50kpps and 100kpps problem is gone.
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On 2015-12-07 12:42, Martin Pieuchot wrote: On 07/12/15(Mon) 00:36, Mickael Torres wrote: On 2015-12-06 20:10, Ian Darwin wrote: >On 2015-12-06 12:23 PM, Stuart Henderson wrote: >>On 2015/12/06 06:02, Mickael Torres wrote: >>>Hello, >>> >>>This is a kernel patch plus a utility called ugenctl I use to allow >>>selected USB devices to attach as ugen(4) instead of their more >>>specific >>>driver. My use case is a Microchip "PICkit 2 Microcontroller >>>Programmer" >>>that attaches as uhid(4), but the command line utility pk2cmd wants a >>>udev(4) device. There maybe are some other use cases. >>If a device is generally pointless with uhid, we can just knock it >>out completely in the kernel, see the UQ_BAD_HID mechanism. I think this >>applies to your device. >> >>The bigger problem is "dual use" devices; e.g. some want UPS to attach >>to upd(4), others want ugen(4) for use with NUT/apcupsd. Your code is >>partially useful for these, but because it just changes things at >>attach, >>won't survive a reboot - if it could instead force a device to detach >>and reattach to ugen it would help a lot more cases. >> >One fairly common (I believe) use is with USB printers that attach as ulpt >but CUPS wants as ugen - for these, this is useful as it stands - just run >it >from rc. Do people want their UPS to sometimes be upd and other times be >a ugen? I can see there might be "I want to change this now" cases, but I >suspect the majority could be done once at each boot and be quite useful. Hello, Hello Mickael, an updated version that detach/reattach if a device is already connected, when adding and when removing from the ugen list. Seems to work well, but I don't have a lot of hardware to test it. What do you think ? I'd really prefer a solution that doesn't need any button. In other words to always be able to use your device from userland if the kernel is not using it. One way would be to always attach a ugen(4) driver to every USB device, I think that's what FreeBSD does. The other way would be to use the usb(4) bus interface to talk to your device. In both cases some work is needed to guarantee that an endpoint is only driven by one driver at a time. Right now this is done by attaching only one driver. To which driver your device currently attaches? What kind of transfers to you want to submit from userland? The libusb already allow your to submit(some) control transfers. It would be nice if you could improve this rather than adding a hack like this one. Cheers, Martin Hello Martin, Actually the device attaches as uhid. I don't know which kind of transfer is used, I'll have to look into this, but the soft (pk2cmd) uses libusb and only works when the device is attached as ugen. And another soft (pic32prog) which also uses libusb seems to only detect the device when it is attached as uhid. I'll try to take a look into libusb. Cheers, Mike.
Re: preparing multitouch support - request for tests
On 06/12/15(Sun) 20:47, joshua stein wrote: > On Thu, 03 Dec 2015 at 00:20:15 +0100, Ulf Brosziewski wrote: > > The diffs below contain a complete and extensive rewrite of the > > input-processing parts of wsmouse and the interface it provides to > > the hardware drivers. It prepares the support for various kinds of > > multitouch input, as well as an extended support for touchpads by > > wsmouse. > [...] > I don't really agree with the direction of the trackpad driver, > though. I agree with this direction. > It looks that you want to move from a "dumb" kernel > interface and a "smart" xorg driver (synaptics) to a smart kernel > interface and a dumb xorg driver (ws). Aside from wsmoused being > able to use it, what is the benefit of putting all the logic in the > kernel? - You can easily multiplex input devices because you don't need a userland program to deal with various /dev/wsmouse*. - You don't need to make userland aware that a suspend/resume cycle happened. - You're not dependant from a third-party library developed by people with much more resources and a different agenda. > I've needed to tweak synclient settings on many of my > laptops like scroll speed, multi-finger clicks, palm detection, > finger widths. Will all of that still be possible with your kernel > driver? I think you're asking the wrong question. What you want is sane default with your touchpad. Obviously you currently don't have them and you need to push buttons to make your hardware usable. Do you need a synclient-like in MacOS X? How many buttons do we really need? I'd suggest you to try Ulf's stuff and see what needs to be tuned. If buttons are *really* necessary, then wsconstl(8) would be the way to go.
Re: 3rd party Xbox 360 USB controller support
On 05/12/15(Sat) 15:22, Christian Heckendorf wrote:
> The previous thread[1] discussing these controllers includes two
> patches but they seem to have been merged for the commit in a way
> that limits support to only Microsoft controllers. 3rd party Xbox 360
> controllers have their own vendor and product IDs but use the same
> subclass and protocol as the Microsoft controllers.
>
> Here's a diff based on the first patch that will match controllers
> and assign the report descriptor more generally using subclass/protocol
> rather than vendor/product. Is it more correct to create an array
> of known vendors/products and match against a call to usb_lookup()?
It's fine since the same check is used in uhidev_use_rdesc().
Could you try this on a Microsoft controller? Does it still work?
Jeremy could you check this out?
>
> [1] http://marc.info/?l=openbsd-tech&m=138229619410284&w=2
>
> Thanks,
> Christian
>
>
> Index: uhidev.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 uhidev.c
> --- uhidev.c 28 Feb 2015 08:42:41 - 1.70
> +++ uhidev.c 5 Dec 2015 20:14:49 -
> @@ -62,7 +62,10 @@
> #ifndef SMALL_KERNEL
> /* Replacement report descriptors for devices shipped with broken ones */
> #include
> -int uhidev_use_rdesc(struct uhidev_softc *, int, int, void **, int *);
> +int uhidev_use_rdesc(struct uhidev_softc *, usb_interface_descriptor_t *,
> + int, int, void **, int *);
> +#define UISUBCLASS_XBOX360_CONTROLLER 0x5d
> +#define UIPROTO_XBOX360_GAMEPAD 0x01
> #endif /* !SMALL_KERNEL */
>
> #define DEVNAME(sc) ((sc)->sc_dev.dv_xname)
> @@ -118,10 +121,10 @@ uhidev_match(struct device *parent, void
> if (id == NULL)
> return (UMATCH_NONE);
> #ifndef SMALL_KERNEL
> - if (uaa->vendor == USB_VENDOR_MICROSOFT &&
> - uaa->product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER &&
> - id->bInterfaceNumber == 0)
> - return (UMATCH_VENDOR_PRODUCT);
> + if (id->bInterfaceClass == UICLASS_VENDOR &&
> + id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> + id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)
> + return (UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO);
> #endif /* !SMALL_KERNEL */
> if (id->bInterfaceClass != UICLASS_HID)
> return (UMATCH_NONE);
> @@ -191,7 +194,7 @@ uhidev_attach(struct device *parent, str
> }
>
> #ifndef SMALL_KERNEL
> - if (uhidev_use_rdesc(sc, uaa->vendor, uaa->product, &desc, &size))
> + if (uhidev_use_rdesc(sc, id, uaa->vendor, uaa->product, &desc, &size))
> return;
> #endif /* !SMALL_KERNEL */
>
> @@ -275,8 +278,8 @@ uhidev_attach(struct device *parent, str
>
> #ifndef SMALL_KERNEL
> int
> -uhidev_use_rdesc(struct uhidev_softc *sc, int vendor, int product,
> -void **descp, int *sizep)
> +uhidev_use_rdesc(struct uhidev_softc *sc, usb_interface_descriptor_t *id,
> + int vendor, int product, void **descp, int *sizep)
> {
> static uByte reportbuf[] = {2, 2};
> const void *descptr = NULL;
> @@ -300,8 +303,9 @@ uhidev_use_rdesc(struct uhidev_softc *sc
> default:
> break;
> }
> - } else if (vendor == USB_VENDOR_MICROSOFT &&
> - product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER) {
> + } else if ((id->bInterfaceClass == UICLASS_VENDOR &&
> +id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> +id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)) {
> /* The Xbox 360 gamepad has no report descriptor. */
> size = sizeof(uhid_xb360gp_report_descr);
> descptr = uhid_xb360gp_report_descr;
>
Re: ugenctl for attaching USB devices to ugen instead of their specific driver
On 07/12/15(Mon) 00:36, Mickael Torres wrote: > On 2015-12-06 20:10, Ian Darwin wrote: > >On 2015-12-06 12:23 PM, Stuart Henderson wrote: > >>On 2015/12/06 06:02, Mickael Torres wrote: > >>>Hello, > >>> > >>>This is a kernel patch plus a utility called ugenctl I use to allow > >>>selected USB devices to attach as ugen(4) instead of their more > >>>specific > >>>driver. My use case is a Microchip "PICkit 2 Microcontroller > >>>Programmer" > >>>that attaches as uhid(4), but the command line utility pk2cmd wants a > >>>udev(4) device. There maybe are some other use cases. > >>If a device is generally pointless with uhid, we can just knock it > >>out completely in the kernel, see the UQ_BAD_HID mechanism. I think this > >>applies to your device. > >> > >>The bigger problem is "dual use" devices; e.g. some want UPS to attach > >>to upd(4), others want ugen(4) for use with NUT/apcupsd. Your code is > >>partially useful for these, but because it just changes things at > >>attach, > >>won't survive a reboot - if it could instead force a device to detach > >>and reattach to ugen it would help a lot more cases. > >> > >One fairly common (I believe) use is with USB printers that attach as ulpt > >but CUPS wants as ugen - for these, this is useful as it stands - just run > >it > >from rc. Do people want their UPS to sometimes be upd and other times be > >a ugen? I can see there might be "I want to change this now" cases, but I > >suspect the majority could be done once at each boot and be quite useful. > > Hello, Hello Mickael, > an updated version that detach/reattach if a device is already connected, > when > adding and when removing from the ugen list. > Seems to work well, but I don't have a lot of hardware to test it. > What do you think ? I'd really prefer a solution that doesn't need any button. In other words to always be able to use your device from userland if the kernel is not using it. One way would be to always attach a ugen(4) driver to every USB device, I think that's what FreeBSD does. The other way would be to use the usb(4) bus interface to talk to your device. In both cases some work is needed to guarantee that an endpoint is only driven by one driver at a time. Right now this is done by attaching only one driver. To which driver your device currently attaches? What kind of transfers to you want to submit from userland? The libusb already allow your to submit(some) control transfers. It would be nice if you could improve this rather than adding a hack like this one. Cheers, Martin
Re: Overflowable int -> size_t in grep
On Mon, Dec 07, 2015 at 02:32:50AM -0500, Michael McConville wrote:
> Otto Moerbeek wrote:
> > On Mon, Dec 07, 2015 at 01:36:22AM -0500, Michael McConville wrote:
> > > This isn't a grave issue, but I came across it while exploring integer
> > > overflow and think it's worth sharing.
> > >
> > > grep represents line numbers with an int, which predictably overflows
> > > for inputs with >= 2^31 newlines. This is easy to demonstrate using the
> > > -n option and a debugging printf.
> > >
> > > The below diff fixes this, and tunes up a for loop while I'm in there.
> >
> > how does this fix work on 32-bit platforms? Better use offset_t.
>
> Good catch, thanks.
>
> Does this look better? Or is PRId64 preferred for off_t?
I don't think off_t is related to ptrdiff_t. The only thing defined for
off_t is that it's a signed integer type, meant to express offsets in a file.
Just cast to long long and use %lld.
And please remove the unrelated for loop change.
-Otto
>
>
> Index: grep.h
> ===
> RCS file: /cvs/src/usr.bin/grep/grep.h,v
> retrieving revision 1.22
> diff -u -p -r1.22 grep.h
> --- grep.h16 Mar 2015 13:26:52 - 1.22
> +++ grep.h7 Dec 2015 07:30:52 -
> @@ -43,7 +43,7 @@
>
> typedef struct {
> size_t len;
> - int line_no;
> + off_tline_no;
> off_toff;
> char*file;
> char*dat;
> Index: util.c
> ===
> RCS file: /cvs/src/usr.bin/grep/util.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 util.c
> --- util.c25 Jun 2015 02:04:08 - 1.50
> +++ util.c7 Dec 2015 07:30:52 -
> @@ -124,7 +124,7 @@ procfile(char *fn)
>
> if (Bflag > 0)
> initqueue();
> - for (c = 0; c == 0 || !(lflag || qflag); ) {
> + for (c = 0; c == 0 || !(lflag || qflag); c += t) {
> ln.off += ln.len + 1;
> if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL)
> break;
> @@ -138,7 +138,6 @@ procfile(char *fn)
> enqueue(&ln);
> linesqueued++;
> }
> - c += t;
> }
> if (Bflag > 0)
> clearqueue();
> @@ -623,7 +622,7 @@ printline(str_t *line, int sep, regmatch
> if (nflag) {
> if (n)
> putchar(sep);
> - printf("%d", line->line_no);
> + printf("%jd", line->line_no);
> ++n;
> }
> if (bflag) {
Re: Overflowable int -> size_t in grep
On Mon, Dec 07, 2015 at 02:32:50AM -0500, Michael McConville wrote: > Otto Moerbeek wrote: > > On Mon, Dec 07, 2015 at 01:36:22AM -0500, Michael McConville wrote: > > > This isn't a grave issue, but I came across it while exploring integer > > > overflow and think it's worth sharing. > > > > > > grep represents line numbers with an int, which predictably overflows > > > for inputs with >= 2^31 newlines. This is easy to demonstrate using the > > > -n option and a debugging printf. > > > > > > The below diff fixes this, and tunes up a for loop while I'm in there. > > > > how does this fix work on 32-bit platforms? Better use offset_t. > > Good catch, thanks. > > Does this look better? Or is PRId64 preferred for off_t? Neither is correct, off_t should be cast to intmax_t first and then %jd be used. Joerg
Re: taskctx and revisiting if_start serialisation
On Sun, Dec 06, 2015 at 03:51:26PM +0100, Martin Pieuchot wrote: > On 06/12/15(Sun) 14:00, David Gwynne wrote: > > the current code for serialising if_start calls for mpsafe nics does > > what it says. > > > > however, kettenis realised it doesnt help us much when we're trying > > to coordinate between the start and txeof side of a driver when > > setting or clearing oactive. in particular, a start routine can > > figure out there's no more space, and then set oactive. txeof could > > be running on another cpu emptying the ring and clearing it. if > > that clear runs in between the other cpus space check and > > ifq_set_oactive, then the nic will be marked full and the stack > > wont ever call start on it again. > > > > so it can be argued that start and txeof should be serialised. > > indeed, other platforms do exactly that. > > > > the least worst mechanism we have for doing that is taskqs. however, > > all my experiments deferring start to a taskq end up significantly > > hurting performance. > > [...] > > while toying with ideas on how to solve kettenis' oactive problem, > > i came up with the following. > > > > it combines tasks with direct dispatch, and borrows the current > > ifq_serialiser/pool/scsi serialisation algorithm. > > I like the idea. \o/ > > the idea is you have a taskctx, which represents a serialising > > context for tasks. tasks are submitted to the taskctx, and the code > > will try to run the tasks immediately rather than defer them to a > > thread. if there is contention on the context, the contending cpu > > yields after queueing the task because the other cpu is responsible > > for running all pending tasks to completion. > > > > it also simplifies the barrier operations a lot. > > > > the diff below implements a generic taskctx framework, and cuts the > > mpsafe if_start() implementation over to it. > > I'm not sure this should be implemented as a generic framework. I'd > suggest to keep it specific to if_start(). As you say above the least > worst mechanism we have is currently taskqs, but maybe this could/will > change? im in two minds about where the ctx code should sit. i obviously lean toward keeping it with taskqs, but i also cant come up with another use case for it. i also havent tried very hard to think of one. are you asking if taskqs will get better, or if something better than taskqs will come along? > I cannot understand what's happening by reading the myx(4) chunk itself. > So I believe the interface needs to be polished. Would it be possible > to implement this serialization without introducing if_restart()? that is by far my least favourite bit of this diff. in my defense, i wrote it while my mother was trying to have a conversation with me, so it didn't get my full attention. ive torn if_restart out and implemented it completely in myx below. > > myx is also changed to only clr oactive from within the taskctx > > serialiser, thereby avoiding the race, but keeps the bulk of txeof > > outside the serialiser so it can run concurrently with start. > > > > other nics are free to serialise start and txeof within the > > ifq_serializer if they want, or not, it is up to them. > > > > thoughts? tests? opinions on messy .h files? > > It appears to me that you have unrelated changes: if_enter/if_leave. oops, yes. i suck at juggling diffs. maybe we should call if.c full and split it up ;) Index: share/man/man9/Makefile === RCS file: /cvs/src/share/man/man9/Makefile,v retrieving revision 1.262 diff -u -p -r1.262 Makefile --- share/man/man9/Makefile 25 Nov 2015 03:09:57 - 1.262 +++ share/man/man9/Makefile 7 Dec 2015 06:30:42 - @@ -397,9 +397,14 @@ MLINKS+=systrace.9 systrace_redirect.9 \ systrace.9 systrace_fork.9 systrace.9 systrace_exit.9 MLINKS+=task_add.9 taskq_create.9 \ task_add.9 taskq_destroy.9 \ + task_add.9 taskq_barrier.9 \ task_add.9 task_set.9 \ task_add.9 task_del.9 \ - task_add.9 TASK_INITIALIZER.9 + task_add.9 TASK_INITIALIZER.9 \ + task_add.9 taskctx_init.9 \ + task_add.9 TASKCTX_INITIALIZER.9 \ + task_add.9 taskctx_barrier.9 \ + task_add.9 task_dispatch.9 MLINKS+=time.9 boottime.9 time.9 mono_time.9 time.9 runtime.9 MLINKS+=timeout.9 timeout_add.9 timeout.9 timeout_set.9 \ timeout.9 timeout_pending.9 timeout.9 timeout_del.9 \ Index: share/man/man9/task_add.9 === RCS file: /cvs/src/share/man/man9/task_add.9,v retrieving revision 1.16 diff -u -p -r1.16 task_add.9 --- share/man/man9/task_add.9 14 Sep 2015 15:14:55 - 1.16 +++ share/man/man9/task_add.9 7 Dec 2015 06:30:42 - @@ -18,15 +18,23 @@ .Dt TASK_ADD 9 .Os .Sh NAME +.Nm task_set , +.Nm TASK_INITIALIZER , .Nm taskq_create , .Nm taskq_destroy , -.Nm task_set , +.Nm taskq_barrier , .Nm task_add , .Nm task_del , -.Nm TASK_INITIALIZER
Re: [patch] Convert modulus to arc4random_uniform
On Mon, Dec 07, 2015 at 12:49:17AM -0600, Matthew Martin wrote:
>
> Theo's diff inspired me to look for other cases of modulo bias. The
> following diff converts most modulus operations on a random number to
> use arc4random_uniform or & as appropriate. I excluded
>
> lib/libsqlite3/src/resolve.c
> regress/lib/libevent/test-time.c
> usr.sbin/nsd/rrl.c
> usr.sbin/nsd/util.c
> usr.sbin/nsd/xfrd.c
>
> as they seem to have upstreams. The only other case is games/wump/wump.c
> which has
>
> if (arc4random() % 2 == 1)
>
> This is safe and seems obvious enough to me.
>
> - Matthew Martin
Thank you. I was going to do the same :)
I think some of these are ok, but I'm unsure about some of the others.
Here are some of my concerns:
- since arc4random_uniform can potentially loop indefinitely, it
might interfere with predictable timing of some routines. I can't
tell if this is harmless in all cases below. This applies in
particular to the proposed changes in the kernel.
- changing random() to arc4random() in games might have undesired
side-effects like interfering with the reproducibility of tests or
games. I think this might apply to awk for the same reason as in the
shells.
> Index: games/atc/update.c
> ===
ok
> Index: games/hack/hack.mklev.c
> ===
> Index: games/hack/rnd.c
> ===
unsure about these two
> Index: lib/libc/stdlib/rand.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/rand.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 rand.c
> --- lib/libc/stdlib/rand.c13 Sep 2015 08:31:47 - 1.15
> +++ lib/libc/stdlib/rand.c7 Dec 2015 06:42:17 -
> @@ -50,7 +50,7 @@ int
> rand(void)
> {
> if (rand_deterministic == 0)
> - return (arc4random() % ((u_int)RAND_MAX + 1));
> + return (arc4random_uniform((u_int)RAND_MAX + 1));
> return (rand_r(&next));
> }
>
this is modulo 2^n, so I think this one is fine as it is.
> Index: sbin/dhclient/dhclient.c
> ===
I have already done this independently.
> Index: sys/dev/ic/sili.c
> ===
> Index: sys/netinet6/nd6_rtr.c
> ===
> Index: sys/ufs/ffs/ffs_alloc.c
> ===
These must definitely be looked at by somebody else.
> Index: usr.bin/awk/run.c
> ===
Unsure about this one. I think deterministic sequences might be desired
in some circumstances (this one is deterministic when a seed was given).
> Index: usr.sbin/npppd/common/slist.c
> ===
> Index: usr.sbin/npppd/l2tp/l2tpd.c
> ===
> Index: usr.sbin/npppd/pppoe/pppoed.c
> ===
> Index: usr.sbin/npppd/pptp/pptpd.c
> ===
These four are ok with me.
