from:"Robert Millan"

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

2014-03-04 Thread Robert Millan

On 27/02/2014 06:13, Guillem Jover wrote:
>> This change pessimizes on the common case by adding
>> two extra in-loop branches, without actually saving any cost since
>> the allocation needs to happen anyway.
> 
> You mean on a loop that is performing a syscall per byte, ok.

I don't think you should speak of Baptiste's code this way. Sure, there's
room for improvement. And sure, the read() could use a larger chunk size
since the buffer allows this, saving a lot of syscalls.

But your statement is akin to "this code performs so bad, it doesn't matter
if we make it even worse".

This isn't the right way to judge someone else's work, and it isn't the
right way to think of problems either. We should strive to solve them (in
this case, making keyboards and mice work again!), rather than to make
them worse.

> Anyway, I'm not the one who's going to merge such changes, so…

I expected as much.

I think the code is in good shape now. I already took advantage of the
useful parts in your review and turned them into improvements in the actual
code. I will now wait for someone with the authority to either approve or
reject it before taking further action.

-- 
Robert Millan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

2014-02-24 Thread Robert Millan

; +
>> +for (i = 0; hw_types[i].driver != NULL; i++) {
>> +for (j = 0; sysctl_exists("dev.%s.%i.%%desc", hw_types[i].driver, 
>> j); j++) {
>> +snprintf(devicename, 1024, "%s%i", hw_types[i].driver, j);
> 
> sizeof(devicename) instead of hard-coding 1024, which can easily get
> out of sync.

Fixed, thanks.

>> +if (connect(sock_devd, (struct sockaddr *) &devd, sizeof(struct 
>> sockaddr_un)) < 0) {
> 
> Better sizeof(devd), it will always match the variable type,
> and it's shorter so the line fits in 80 chars.

Fixed, thanks.

>> -#if defined(CONFIG_HAL) || defined(CONFIG_UDEV) || defined(CONFIG_WSCONS)
>> +#if defined(CONFIG_HAL) || defined(CONFIG_UDEV) || defined(CONFIG_WSCONS) 
>> || defined(CONFIG_DEVD)
>>  .forceInputDevices = FALSE,
>>  .autoAddDevices = TRUE,
>>  .autoEnableDevices = TRUE,
> 
> You might want to wrap these long lines to 80 chars.

Done.

On 22/02/2014 17:15, Guillem Jover wrote:
>>> +static void
>>> +device_added(char *line)
>>> +{
> 
>>> +vendor = sysctl_get_str("dev.%s.%s.%%desc", hw_types[i].driver, line + 
>>> strlen(hw_types[i].driver));
>>> +if (vendor == NULL) {
>>> +attrs.vendor = strdup("(unnamed)");
>>> +} else {
>>> +if ((product = strchr(vendor, ' ')) != NULL) {
>>> +product[0] = '\0';
>>> +product++;
>>> +}
>>> +attrs.vendor = strdup(vendor);
>>> +if (product != NULL && (walk = strchr(product, ',')) != NULL)
>>> +walk[0] = '\0';
>>> +attrs.product = strdup(product != NULL ? product : "(unnamed)");
>>> +options = input_option_new(options, "name", product != NULL ? 
>>> product : "(unnamed)");
>>> +}
>>> +attrs.usb_id = NULL;
>>> +attrs.device = strdup(path);
>>> +options = input_option_new(options, "driver", hw_types[i].xdriver);
> 
>>> +rc = NewInputDeviceRequest(options, &attrs, &dev);
> 
> Ah, missed this one, you might want to check how NewInputDeviceRequest()
> copes with NULL attributes if strdup() fails, and if you might need to
> bail on one of those strdup() failures.

Looks safe to me. The codepaths lead to MatchAttrToken() and 
DuplicateInputAttributes(), neither
of which assumes any of the attributes to be non-NULL.

-- 
Robert Millan
>From d96e2bd2a2b48ede527ad7071d3e0eeda9861b73 Mon Sep 17 00:00:00 2001
From: Robert Millan 
Date: Mon, 24 Feb 2014 23:22:57 +0100
Subject: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

Based on original code by Baptiste Daroussin, with some fixes made
by Koop Mast and myself.

Signed-off-by: Robert Millan 
---
 config/Makefile.am  |4 +
 config/config-backends.h|5 +
 config/config.c |5 +
 config/devd.c   |  387 +++
 configure.ac|   16 ++
 hw/xfree86/common/xf86Config.c  |7 +-
 hw/xfree86/common/xf86Globals.c |3 +-
 include/dix-config.h.in |3 +
 8 files changed, 427 insertions(+), 3 deletions(-)
 create mode 100644 config/devd.c

diff --git a/config/Makefile.am b/config/Makefile.am
index 0e20e8b..16f8aaa 100644
--- a/config/Makefile.am
+++ b/config/Makefile.am
@@ -34,6 +34,10 @@ if CONFIG_WSCONS
 libconfig_la_SOURCES += wscons.c
 endif # CONFIG_WSCONS
 
+if CONFIG_DEVD
+libconfig_la_SOURCES += devd.c
+endif
+
 endif # !CONFIG_HAL
 
 endif # !CONFIG_UDEV
diff --git a/config/config-backends.h b/config/config-backends.h
index 5f07557..889e2ad 100644
--- a/config/config-backends.h
+++ b/config/config-backends.h
@@ -44,3 +44,8 @@ void config_hal_fini(void);
 int config_wscons_init(void);
 void config_wscons_fini(void);
 #endif
+
+#ifdef CONFIG_DEVD
+int config_devd_init(void);
+void config_devd_fini(void);
+#endif
diff --git a/config/config.c b/config/config.c
index 760cf19..cca5a31 100644
--- a/config/config.c
+++ b/config/config.c
@@ -53,6 +53,9 @@ config_init(void)
 #elif defined(CONFIG_WSCONS)
 if (!config_wscons_init())
 ErrorF("[config] failed to initialise wscons\n");
+#elif defined(CONFIG_DEVD)
+if (!config_devd_init())
+ErrorF("[config] failed to initialise devd\n");
 #endif
 }
 
@@ -65,6 +68,8 @@ config_fini(void)
 config_hal_fini();
 #elif defined(CONFIG_WSCONS)
 config_wscons_fini();
+#elif defined(CONFIG_DEVD)
+config_devd_fini();
 #endif
 }
 
diff --git a/config/devd.c b/config/devd.c
new file mode 100644
index 000..92a66c4
--- /dev/null
+++ b/config/devd.c
@@ -0,0 +1,387 @@
+/*
+ * Copyright Â© 201

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

2014-02-21 Thread Robert Millan


Anyone who could commit this for me?

On 16/02/2014 12:17, Robert Millan wrote:
> On 16/02/2014 05:10, Alexander E. Patrakov wrote:
>>
>> "Robert Millan" mailto:r...@debian.org>> wrote:
>>
>>> (a patch)
>>
>> The patch uses the "pointer" type, which is on its way out. Please replace 
>> with "void *".
> 
> Here.
> 
> 
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 


-- 
Robert Millan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

2014-02-16 Thread Robert Millan

On 16/02/2014 05:10, Alexander E. Patrakov wrote:
> 
> "Robert Millan" mailto:r...@debian.org>> wrote:
> 
>> (a patch)
> 
> The patch uses the "pointer" type, which is on its way out. Please replace 
> with "void *".

Here.

-- 
Robert Millan
>From 34f6395846f88abec1ae72a74fd0aa35ec1f99a7 Mon Sep 17 00:00:00 2001
From: Robert Millan 
Date: Sun, 16 Feb 2014 12:14:42 +
Subject: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

Based on original code by Baptiste Daroussin, with some fixes made
by Koop Mast and myself.

Signed-off-by: Robert Millan 
---
 config/Makefile.am  |4 +
 config/config-backends.h|5 +
 config/config.c |5 +
 config/devd.c   |  365 +++
 configure.ac|   16 ++
 hw/xfree86/common/xf86Config.c  |6 +-
 hw/xfree86/common/xf86Globals.c |2 +-
 include/dix-config.h.in |3 +
 8 files changed, 403 insertions(+), 3 deletions(-)
 create mode 100644 config/devd.c

diff --git a/config/Makefile.am b/config/Makefile.am
index c6350be..4471d77 100644
--- a/config/Makefile.am
+++ b/config/Makefile.am
@@ -34,6 +34,10 @@ if CONFIG_WSCONS
 libconfig_la_SOURCES += wscons.c
 endif # CONFIG_WSCONS
 
+if CONFIG_DEVD
+libconfig_la_SOURCES += devd.c
+endif
+
 endif # !CONFIG_HAL
 
 endif # !CONFIG_UDEV
diff --git a/config/config-backends.h b/config/config-backends.h
index 5f07557..889e2ad 100644
--- a/config/config-backends.h
+++ b/config/config-backends.h
@@ -44,3 +44,8 @@ void config_hal_fini(void);
 int config_wscons_init(void);
 void config_wscons_fini(void);
 #endif
+
+#ifdef CONFIG_DEVD
+int config_devd_init(void);
+void config_devd_fini(void);
+#endif
diff --git a/config/config.c b/config/config.c
index 760cf19..cca5a31 100644
--- a/config/config.c
+++ b/config/config.c
@@ -53,6 +53,9 @@ config_init(void)
 #elif defined(CONFIG_WSCONS)
 if (!config_wscons_init())
 ErrorF("[config] failed to initialise wscons\n");
+#elif defined(CONFIG_DEVD)
+if (!config_devd_init())
+ErrorF("[config] failed to initialise devd\n");
 #endif
 }
 
@@ -65,6 +68,8 @@ config_fini(void)
 config_hal_fini();
 #elif defined(CONFIG_WSCONS)
 config_wscons_fini();
+#elif defined(CONFIG_DEVD)
+config_devd_fini();
 #endif
 }
 
diff --git a/config/devd.c b/config/devd.c
new file mode 100644
index 000..68e658a
--- /dev/null
+++ b/config/devd.c
@@ -0,0 +1,365 @@
+/*
+ * Copyright Â© 2012 Baptiste Daroussin
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author: Baptiste Daroussin 
+ */
+
+#ifdef HAVE_DIX_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "input.h"
+#include "inputstr.h"
+#include "hotplug.h"
+#include "config-backends.h"
+#include "os.h"
+
+#define DEVD_SOCK_PATH "/var/run/devd.pipe"
+
+#define DEVD_EVENT_ADD		'+'
+#define DEVD_EVENT_REMOVE	'-'
+
+static int sock_devd = -1;
+
+struct hw_type {
+	const char *driver;
+	int flag;
+	const char *xdriver;
+};
+
+static struct hw_type hw_types[] = {
+	{ "ukbd", ATTR_KEYBOARD, "kbd" },
+	{ "atkbd", ATTR_KEYBOARD, "kbd" },
+	{ "ums", ATTR_POINTER, "mouse" },
+	{ "psm", ATTR_POINTER, "mouse" },
+	{ "uhid", ATTR_POINTER, "mouse" },
+	{ "joy", ATTR_JOYSTICK, NULL },
+	{ "atp", ATTR_TOUCHPAD, NULL },
+	{ "uep", ATTR_TOUCHSCREEN, NULL },
+	{ NULL, -1, NULL },
+};
+
+static bool
+sysctl_exists(const char *format, ...)
+{
+	va_list args;
+	char *name = NULL;
+	size_t len;
+	int ret;
+
+	if (format == NULL)
+		retu

[PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

2014-02-15 Thread Robert Millan

>From b353bbf362f5d8ca1b9775f8c8cae9f6950f3d19 Mon Sep 17 00:00:00 2001
From: Robert Millan 
Date: Sat, 15 Feb 2014 22:16:25 +
Subject: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

Based on original code by Baptiste Daroussin, with some fixes made
by Koop Mast and myself.

Signed-off-by: Robert Millan 
---
 config/Makefile.am  |4 +
 config/config-backends.h|5 +
 config/config.c |5 +
 config/devd.c   |  365 +++
 configure.ac|   16 ++
 hw/xfree86/common/xf86Config.c  |6 +-
 hw/xfree86/common/xf86Globals.c |2 +-
 include/dix-config.h.in |3 +
 8 files changed, 403 insertions(+), 3 deletions(-)
 create mode 100644 config/devd.c

diff --git a/config/Makefile.am b/config/Makefile.am
index c6350be..4471d77 100644
--- a/config/Makefile.am
+++ b/config/Makefile.am
@@ -34,6 +34,10 @@ if CONFIG_WSCONS
 libconfig_la_SOURCES += wscons.c
 endif # CONFIG_WSCONS
 
+if CONFIG_DEVD
+libconfig_la_SOURCES += devd.c
+endif
+
 endif # !CONFIG_HAL
 
 endif # !CONFIG_UDEV
diff --git a/config/config-backends.h b/config/config-backends.h
index 5f07557..889e2ad 100644
--- a/config/config-backends.h
+++ b/config/config-backends.h
@@ -44,3 +44,8 @@ void config_hal_fini(void);
 int config_wscons_init(void);
 void config_wscons_fini(void);
 #endif
+
+#ifdef CONFIG_DEVD
+int config_devd_init(void);
+void config_devd_fini(void);
+#endif
diff --git a/config/config.c b/config/config.c
index 760cf19..cca5a31 100644
--- a/config/config.c
+++ b/config/config.c
@@ -53,6 +53,9 @@ config_init(void)
 #elif defined(CONFIG_WSCONS)
 if (!config_wscons_init())
 ErrorF("[config] failed to initialise wscons\n");
+#elif defined(CONFIG_DEVD)
+if (!config_devd_init())
+ErrorF("[config] failed to initialise devd\n");
 #endif
 }
 
@@ -65,6 +68,8 @@ config_fini(void)
 config_hal_fini();
 #elif defined(CONFIG_WSCONS)
 config_wscons_fini();
+#elif defined(CONFIG_DEVD)
+config_devd_fini();
 #endif
 }
 
diff --git a/config/devd.c b/config/devd.c
new file mode 100644
index 000..c79c76d
--- /dev/null
+++ b/config/devd.c
@@ -0,0 +1,365 @@
+/*
+ * Copyright © 2012 Baptiste Daroussin
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author: Baptiste Daroussin 
+ */
+
+#ifdef HAVE_DIX_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "input.h"
+#include "inputstr.h"
+#include "hotplug.h"
+#include "config-backends.h"
+#include "os.h"
+
+#define DEVD_SOCK_PATH "/var/run/devd.pipe"
+
+#define DEVD_EVENT_ADD		'+'
+#define DEVD_EVENT_REMOVE	'-'
+
+static int sock_devd = -1;
+
+struct hw_type {
+	const char *driver;
+	int flag;
+	const char *xdriver;
+};
+
+static struct hw_type hw_types[] = {
+	{ "ukbd", ATTR_KEYBOARD, "kbd" },
+	{ "atkbd", ATTR_KEYBOARD, "kbd" },
+	{ "ums", ATTR_POINTER, "mouse" },
+	{ "psm", ATTR_POINTER, "mouse" },
+	{ "uhid", ATTR_POINTER, "mouse" },
+	{ "joy", ATTR_JOYSTICK, NULL },
+	{ "atp", ATTR_TOUCHPAD, NULL },
+	{ "uep", ATTR_TOUCHSCREEN, NULL },
+	{ NULL, -1, NULL },
+};
+
+static bool
+sysctl_exists(const char *format, ...)
+{
+	va_list args;
+	char *name = NULL;
+	size_t len;
+	int ret;
+
+	if (format == NULL)
+		return false;
+
+	va_start(args, format);
+	vasprintf(&name, format, args);
+	va_end(args);
+
+	ret = sysctlbyname(name, NULL, &len, NULL, 0);
+
+	if (ret == -1)
+		len = 0;
+
+	free(name);
+	return (len > 0);
+}
+
+static char *
+sysctl_get_str(const char *format, ...)
+{
+	va_list args

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

Re: [PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

[PATCH] Add devd config backend for FreeBSD (and GNU/kFreeBSD)

5 matches

Site Navigation

Mail list logo

Footer information