Two patches

Two patches

[Grigory Batalov]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

   Hello, I made some patches for dosemu-1.1.2 (1.1.3).
 Maybe you'll find them usefull.

 1) dosemu-1.1.2-no_perl.patch removes perl-base dependence
 from dosemu. I replace perl calls with sed and mkcomstub
 script with C program.

 2) dosemu-1.1.2-fonts.patch
 As I'm russian, I use special cp866 X-font with dosemu.
 But I don't want to remove original vga.pcf, so I have
 all of them in ~/dosemu/Xfonts:

-rw-r--r--    1 bga      bga           141 Apr  8 14:06 fonts.alias
-rw-r--r--    1 bga      bga          4747 Apr  8 14:09 vga-cp866.pcf.gz
-rw-r--r--    1 bga      bga          4743 Nov  4  1995 vga.pcf.gz
-rw-r--r--    1 bga      bga          5387 Apr  8 14:09 vga10x20-cp866.pcf.gz

 fonts.alias contains aliases that I use in $_X_font declaration.

 Any comments are welcome.

--
Grigory Batalov.

Re: Two patches

[Grigory Batalov]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

 where are my patches?

--
 Grigory Batalov.

[-- Attachment #2: dosemu-1.1.2-no_perl.patch.gz --]
[-- Type: application/x-gzip, Size: 1859 bytes --]

[-- Attachment #3: dosemu-1.1.2-fonts.patch.gz --]
[-- Type: application/x-gzip, Size: 921 bytes --]

two patches

[Jean Delvare]

Hello

Here are two patches for lm_sensors2 I wrote in the train two hours ago.

The first one fixes a (maybe dangerous) bug in libsensors where
read-only features could be written and write-only features could be
read.

The second one simply (re)indents the eeprom section of prog/sensors. I
plan to make modifications on this part soon, and it was hardly readable
the way it was before.

-- 
Jean "Khali" Delvare
http://www.ensicaen.ismra.fr/~delvare/
-------------- next part --------------
diff -ruN lm_sensors2/lib/access.c lm_sensors2-and/lib/access.c
--- lm_sensors2/lib/access.c	Fri Dec 24 23:44:40 1999
+++ lm_sensors2-and/lib/access.c	Sun Jul 14 22:06:34 2002
@@ -222,7 +222,7 @@
                    sensors_lookup_feature_nr(name.prefix,
                                              main_feature->compute_mapping)))
     return -SENSORS_ERR_NO_ENTRY;
-  if (! (main_feature->mode && SENSORS_MODE_R))
+  if (! (main_feature->mode & SENSORS_MODE_R))
     return -SENSORS_ERR_ACCESS;
   for (chip = NULL; !expr && (chip = sensors_for_all_config_chips(name,chip));)
     for (i = 0; !final_expr && (i < chip->computes_count); i++) {
@@ -265,7 +265,7 @@
                    sensors_lookup_feature_nr(name.prefix,
                                              main_feature->compute_mapping)))
     return -SENSORS_ERR_NO_ENTRY;
-  if (! (main_feature->mode && SENSORS_MODE_W))
+  if (! (main_feature->mode & SENSORS_MODE_W))
     return -SENSORS_ERR_ACCESS;
   for (chip = NULL; !expr && (chip = sensors_for_all_config_chips(name,chip));)
     for (i = 0; !final_expr && (i < chip->computes_count); i++)
-------------- next part --------------
diff -ruN lm_sensors2/prog/sensors/chips.c lm_sensors2-indent-eeprom/prog/sensors/chips.c
--- lm_sensors2/prog/sensors/chips.c	Fri Jul 12 13:08:06 2002
+++ lm_sensors2-indent-eeprom/prog/sensors/chips.c	Sun Jul 14 22:14:30 2002
@@ -2423,46 +2423,45 @@
 
 void print_eeprom(const sensors_chip_name *name)
 {
-	char  *label = NULL;
-	double a, b, c, d;
-	int    valid, i;
+  char *label = NULL;
+  double a, b, c, d;
+  int valid, i;
 
-   if (!sensors_get_label_and_valid(*name, SENSORS_EEPROM_TYPE, &label,&valid) &&
-       !sensors_get_feature(*name, SENSORS_EEPROM_TYPE, &a)) {
-      if (valid) {
-	if(((int) a) = 4) {
-           print_label(label, 24);
-	   printf("SDRAM DIMM SPD\n");
-	} else if(((int) a) = 7) {
-           print_label(label, 24);
-	   printf("DDR SDRAM DIMM SPD\n");
-	} else
-	      return;
-      }
-   } else
-      printf("ERROR: data 1\n");
-   free_the_label(&label);
+  if (!sensors_get_label_and_valid(*name, SENSORS_EEPROM_TYPE, &label,&valid) &&
+      !sensors_get_feature(*name, SENSORS_EEPROM_TYPE, &a)) {
+    if (valid) {
+      if(((int) a) = 4) {
+        print_label(label, 24);
+        printf("SDRAM DIMM SPD\n");
+      } else if(((int) a) = 7) {
+        print_label(label, 24);
+        printf("DDR SDRAM DIMM SPD\n");
+      } else
+        return;
+    }
+  } else
+    printf("ERROR: data 1\n");
+  free_the_label(&label);
 
-   if (!sensors_get_label_and_valid(*name, SENSORS_EEPROM_ROWADDR, &label,&valid) &&
-       !sensors_get_feature(*name, SENSORS_EEPROM_ROWADDR, &a) &&
-       !sensors_get_feature(*name, SENSORS_EEPROM_COLADDR, &b) &&
-       !sensors_get_feature(*name, SENSORS_EEPROM_NUMROWS, &c) &&
-       !sensors_get_feature(*name, SENSORS_EEPROM_BANKS, &d)) {
-      if (valid) {
-         print_label(label, 24);
-	 i = (((int) a) & 0x0f) + (((int) b) & 0x0f) - 17;
-	 if(i > 0 && i <= 12 && c <= 8 && d <= 8)
-	         printf("%d\n", (1 << i) * ((int) c) * ((int) d));
-	 else
-{
-	         printf("invalid\n");
-printf("%d %d %d %d\n", (int) a, (int) b, (int) c, (int) d);
-}
+  if (!sensors_get_label_and_valid(*name, SENSORS_EEPROM_ROWADDR, &label,&valid) &&
+      !sensors_get_feature(*name, SENSORS_EEPROM_ROWADDR, &a) &&
+      !sensors_get_feature(*name, SENSORS_EEPROM_COLADDR, &b) &&
+      !sensors_get_feature(*name, SENSORS_EEPROM_NUMROWS, &c) &&
+      !sensors_get_feature(*name, SENSORS_EEPROM_BANKS, &d)) {
+    if (valid) {
+      print_label(label, 24);
+      i = (((int) a) & 0x0f) + (((int) b) & 0x0f) - 17;
+      if(i > 0 && i <= 12 && c <= 8 && d <= 8)
+        printf("%d\n", (1 << i) * ((int) c) * ((int) d));
+      else
+      {
+        printf("invalid\n");
+        printf("%d %d %d %d\n", (int) a, (int) b, (int) c, (int) d);
       }
-   } else
-      printf("ERROR: data 2\n");
-   free_the_label(&label);
-
+    }
+  } else
+    printf("ERROR: data 2\n");
+  free_the_label(&label);
 }
 
 

two patches

[Mark D. Studebaker]

I checked in the first one. Good catch.
Not really dangerous because the file modes in /proc of course
overrule whatever's in the library.

Let's hold off on the sensors change until after the release.

mds



Jean Delvare wrote:
> 
> Hello
> 
> Here are two patches for lm_sensors2 I wrote in the train two hours ago.
> 
> The first one fixes a (maybe dangerous) bug in libsensors where
> read-only features could be written and write-only features could be
> read.
> 
> The second one simply (re)indents the eeprom section of prog/sensors. I
> plan to make modifications on this part soon, and it was hardly readable
> the way it was before.
> 
> --
> Jean "Khali" Delvare
> http://www.ensicaen.ismra.fr/~delvare/
> 
> 
>                           Name: lm_sensors-and.diff
>    lm_sensors-and.diff    Type: Plain Text (text/plain)
>                       Encoding: base64
> 
>                                     Name: lm_sensors-indent-eeprom.diff
>    lm_sensors-indent-eeprom.diff    Type: Plain Text (text/plain)
>                                 Encoding: base64

two patches

[Yury G. Kudryashov]

[-- Attachment #1: Type: text/plain, Size: 41 bytes --]

Hi!

Review the attached patches, please.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Include-linux-types.h.patch --]
[-- Type: text/x-patch; name="0001-Include-linux-types.h.patch", Size: 620 bytes --]

From 5cc42c6f87f690fdf66e29de2a816dab49a119f4 Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 00:21:02 +0400
Subject: [PATCH 1/2] Include linux/types.h

---
 extras/hid2hci/hid2hci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/extras/hid2hci/hid2hci.c b/extras/hid2hci/hid2hci.c
index 0d0a022..839c4fb 100644
--- a/extras/hid2hci/hid2hci.c
+++ b/extras/hid2hci/hid2hci.c
@@ -28,6 +28,7 @@
 #include <string.h>
 #include <getopt.h>
 #include <sys/ioctl.h>
+#include <linux/types.h>
 #include <linux/hiddev.h>
 #include <usb.h>
 
-- 
1.7.0.5



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-with-firmware-path-configure-option.patch --]
[-- Type: text/x-patch; name="0002-Add-with-firmware-path-configure-option.patch", Size: 3237 bytes --]

From f9881e554deafad1301d00e4539a02fe63078c00 Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 09:38:32 +0400
Subject: [PATCH 2/2] Add --with-firmware-path configure option

---
 Makefile.am                |    3 ++-
 configure.ac               |   22 ++++++++++++++++++++++
 extras/firmware/firmware.c |   10 ++++++----
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 68a68d9..f094746 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -156,7 +156,7 @@ EXTRA_DIST += \
 	udev/udevd.xml
 
 %.7 %.8 : %.xml
-	$(XSLTPROC) -o $@ -nonet http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
+	$(XSLTPROC) -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
 
 # ------------------------------------------------------------------------------
 # udev tests
@@ -194,6 +194,7 @@ dist_udevrules_DATA += \
 # ------------------------------------------------------------------------------
 extras_firmware_firmware_SOURCES = extras/firmware/firmware.c
 extras_firmware_firmware_LDADD = libudev/libudev-private.la
+extras_firmware_firmware_CPPFLAGS = $(AM_CPPFLAGS) -DFIRMWARE_PATH="$(FIRMWARE_PATH)"
 dist_udevrules_DATA += extras/firmware/50-firmware.rules
 libexec_PROGRAMS = extras/firmware/firmware
 
diff --git a/configure.ac b/configure.ac
index 492fa02..5defd5c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,27 @@ if test "x$enable_extras" = xyes; then
 		fi])
 	AC_SUBST(PCI_DATABASE)
 
+	AC_ARG_WITH(firmware-path,
+		AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
+		   [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
+		[],
+		[with_firmware_path="/lib/fimware/updates:/lib/fimware"]
+		)
+	[
+	OLD_IFS=$IFS
+	IFS=:
+	for i in $with_firmware_path
+	do
+		if [ "x${FIRMWARE_PATH}" == "x" ]; then
+			FIRMWARE_PATH="\\\"${i}/\\\""
+		else
+			FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
+		fi
+	done
+	IFS=$OLD_IFS
+	]
+	AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
+
 	AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
 	AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
 fi
@@ -144,6 +165,7 @@ AC_MSG_RESULT([
 
 	usb.ids:		${USB_DATABASE}
 	pci.ids:		${PCI_DATABASE}
+	firmware path:		${FIRMWARE_PATH}
 
 	xsltproc:		${XSLTPROC}
 	gperf:			${GPERF}
diff --git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
index 92f0918..76071db 100644
--- a/extras/firmware/firmware.c
+++ b/extras/firmware/firmware.c
@@ -79,10 +79,7 @@ int main(int argc, char **argv)
 		{ "help", no_argument, NULL, 'h' },
 		{}
 	};
-	static const char *searchpath[] = {
-		"/lib/firmware/updates/",
-		"/lib/firmware/"
-	};
+	static const char *searchpath[] = { FIRMWARE_PATH };
 	char fwencpath[UTIL_PATH_SIZE];
 	char misspath[UTIL_PATH_SIZE];
 	char loadpath[UTIL_PATH_SIZE];
@@ -97,6 +94,11 @@ int main(int argc, char **argv)
 	unsigned int i;
 	int rc = 0;
 
+	for (i = 0; i < ARRAY_SIZE(searchpath); i++) {
+		printf("Path %s\n", searchpath[i]);
+	}
+	return 0;
+
 	udev_log_init("firmware");
 
 	for (;;) {
-- 
1.7.0.5

Re: two patches

[Dan Nicholson]

On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
> Hi!
> 
> Review the attached patches, please.

> From 5cc42c6f87f690fdf66e29de2a816dab49a119f4 Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 00:21:02 +0400
> Subject: [PATCH 1/2] Include linux/types.h

1. Prefix the subject with "hid2hci: " so it's clear from the shortlog what
   this affects.

2. Please include some rational in the commit message. What prototype or
   datatype is missing that requires including <linux/types.h>?

> 
> ---
>  extras/hid2hci/hid2hci.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/extras/hid2hci/hid2hci.c b/extras/hid2hci/hid2hci.c
> index 0d0a022..839c4fb 100644
> --- a/extras/hid2hci/hid2hci.c
> +++ b/extras/hid2hci/hid2hci.c
> @@ -28,6 +28,7 @@
>  #include <string.h>
>  #include <getopt.h>
>  #include <sys/ioctl.h>
> +#include <linux/types.h>
>  #include <linux/hiddev.h>
>  #include <usb.h>
>  
> -- 
> 1.7.0.5
> 
> 

> From f9881e554deafad1301d00e4539a02fe63078c00 Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 09:38:32 +0400
> Subject: [PATCH 2/2] Add --with-firmware-path configure option

Again, please include some rational in the commit message. What problem is
this solving?

> 
> ---
>  Makefile.am                |    3 ++-
>  configure.ac               |   22 ++++++++++++++++++++++
>  extras/firmware/firmware.c |   10 ++++++----
>  3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 68a68d9..f094746 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -156,7 +156,7 @@ EXTRA_DIST += \
>  	udev/udevd.xml
>  
>  %.7 %.8 : %.xml
> -	$(XSLTPROC) -o $@ -nonet http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
> +	$(XSLTPROC) -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<

This is an unrelated change. Please remove it from this patch.

>  
>  # ------------------------------------------------------------------------------
>  # udev tests
> @@ -194,6 +194,7 @@ dist_udevrules_DATA += \
>  # ------------------------------------------------------------------------------
>  extras_firmware_firmware_SOURCES = extras/firmware/firmware.c
>  extras_firmware_firmware_LDADD = libudev/libudev-private.la
> +extras_firmware_firmware_CPPFLAGS = $(AM_CPPFLAGS) -DFIRMWARE_PATH="$(FIRMWARE_PATH)"
>  dist_udevrules_DATA += extras/firmware/50-firmware.rules
>  libexec_PROGRAMS = extras/firmware/firmware
>  
> diff --git a/configure.ac b/configure.ac
> index 492fa02..5defd5c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -82,6 +82,27 @@ if test "x$enable_extras" = xyes; then
>  		fi])
>  	AC_SUBST(PCI_DATABASE)
>  
> +	AC_ARG_WITH(firmware-path,
> +		AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
> +		   [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
> +		[],
> +		[with_firmware_path="/lib/fimware/updates:/lib/fimware"]
> +		)
> +	[
> +	OLD_IFS=$IFS
> +	IFS=:
> +	for i in $with_firmware_path
> +	do
> +		if [ "x${FIRMWARE_PATH}" = "x" ]; then
> +			FIRMWARE_PATH="\\\"${i}/\\\""
> +		else
> +			FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
> +		fi
> +	done
> +	IFS=$OLD_IFS
> +	]

I'm not sure what wrapping this whole section in [] quoting does. It looks
like a good handling of the option, though.

> +	AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
> +
>  	AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
>  	AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
>  fi
> @@ -144,6 +165,7 @@ AC_MSG_RESULT([
>  
>  	usb.ids:		${USB_DATABASE}
>  	pci.ids:		${PCI_DATABASE}
> +	firmware path:		${FIRMWARE_PATH}
>  
>  	xsltproc:		${XSLTPROC}
>  	gperf:			${GPERF}
> diff --git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
> index 92f0918..76071db 100644
> --- a/extras/firmware/firmware.c
> +++ b/extras/firmware/firmware.c
> @@ -79,10 +79,7 @@ int main(int argc, char **argv)
>  		{ "help", no_argument, NULL, 'h' },
>  		{}
>  	};
> -	static const char *searchpath[] = {
> -		"/lib/firmware/updates/",
> -		"/lib/firmware/"
> -	};
> +	static const char *searchpath[] = { FIRMWARE_PATH };
>  	char fwencpath[UTIL_PATH_SIZE];
>  	char misspath[UTIL_PATH_SIZE];
>  	char loadpath[UTIL_PATH_SIZE];
> @@ -97,6 +94,11 @@ int main(int argc, char **argv)
>  	unsigned int i;
>  	int rc = 0;
>  
> +	for (i = 0; i < ARRAY_SIZE(searchpath); i++) {
> +		printf("Path %s\n", searchpath[i]);
> +	}
> +	return 0;
> +

I'm pretty sure the program stops doing anything useful with this hunk. :)

>  	udev_log_init("firmware");
>  
>  	for (;;) {
> -- 
> 1.7.0.5

Besides those small issues, both patches seem pretty good to me.

--
Dan

Re: two patches

[Kay Sievers]

On Fri, Apr 16, 2010 at 17:28, Dan Nicholson <dbn.lists@gmail.com> wrote:
> On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -82,6 +82,27 @@ if test "x$enable_extras" = xyes; then
>>               fi])
>>       AC_SUBST(PCI_DATABASE)
>>
>> +     AC_ARG_WITH(firmware-path,
>> +             AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
>> +                [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),

Not sure if we need a list of directories here. Wouldn't the prefix
/lib/firmware be sufficient, the same way we handle all the other
configurable directories?

Kay

Re: two patches

[Dan Nicholson]

On Fri, Apr 16, 2010 at 9:31 AM, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Fri, Apr 16, 2010 at 17:28, Dan Nicholson <dbn.lists@gmail.com> wrote:
>> On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -82,6 +82,27 @@ if test "x$enable_extras" = xyes; then
>>>               fi])
>>>       AC_SUBST(PCI_DATABASE)
>>>
>>> +     AC_ARG_WITH(firmware-path,
>>> +             AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
>>> +                [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
>
> Not sure if we need a list of directories here. Wouldn't the prefix
> /lib/firmware be sufficient, the same way we handle all the other
> configurable directories?

This is just maintaining the current default in firmware.c. The code
already deals with an array of directories, so you might as well let
people build in any crazy path they want.

--
Dan

Re: two patches

[Yury G. Kudryashov]

Dan Nicholson wrote:

>> @@ -156,7 +156,7 @@ EXTRA_DIST += \
>>  udev/udevd.xml
>>  
>>  %.7 %.8 : %.xml
>> -	$(XSLTPROC) -o $@ -nonet
>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>> $<
>> +	$(XSLTPROC) -o $@
>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>> $<
> 
> This is an unrelated change. Please remove it from this patch.
Oops. Sorry. I should've reread the patch before sending.
>> +	[
>> +	OLD_IFS=$IFS
>> +	IFS=:
>> +	for i in $with_firmware_path
>> +	do
>> +		if [ "x${FIRMWARE_PATH}" = "x" ]; then
>> +			FIRMWARE_PATH="\\\"${i}/\\\""
>> +		else
>> +			FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
>> +		fi
>> +	done
>> +	IFS=$OLD_IFS
>> +	]
> 
> I'm not sure what wrapping this whole section in [] quoting does. It looks
> like a good handling of the option, though.
It escapes "if [ ... ]".

Re: two patches

[Dan Nicholson]

On Fri, Apr 16, 2010 at 10:51 AM, Yury G. Kudryashov <urkud@ya.ru> wrote:
> Dan Nicholson wrote:
>
>>> @@ -156,7 +156,7 @@ EXTRA_DIST += \
>>>  udev/udevd.xml
>>>
>>>  %.7 %.8 : %.xml
>>> -    $(XSLTPROC) -o $@ -nonet
>>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>>> $<
>>> +    $(XSLTPROC) -o $@
>>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>>> $<
>>
>> This is an unrelated change. Please remove it from this patch.
> Oops. Sorry. I should've reread the patch before sending.
>>> +    [
>>> +    OLD_IFS=$IFS
>>> +    IFS=:
>>> +    for i in $with_firmware_path
>>> +    do
>>> +            if [ "x${FIRMWARE_PATH}" = "x" ]; then
>>> +                    FIRMWARE_PATH="\\\"${i}/\\\""
>>> +            else
>>> +                    FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
>>> +            fi
>>> +    done
>>> +    IFS=$OLD_IFS
>>> +    ]
>>
>> I'm not sure what wrapping this whole section in [] quoting does. It looks
>> like a good handling of the option, though.
> It escapes "if [ ... ]".

Ah. You can also use 'test "x$FIRMWARE_PATH" = x' to avoid that. Also,
I should mention that you should use '=' instead of '=' since the
former is portable across shells.

--
Dan

Re: two patches

[Yury G. Kudryashov]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

Dan Nicholson wrote:

> On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
>> Hi!
>> 
>> Review the attached patches, please.
> 
>> From 5cc42c6f87f690fdf66e29de2a816dab49a119f4 Mon Sep 17 00:00:00 2001
>> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
>> Date: Fri, 16 Apr 2010 00:21:02 +0400
>> Subject: [PATCH 1/2] Include linux/types.h
> 
> 1. Prefix the subject with "hid2hci: " so it's clear from the shortlog
> what
>    this affects.
> 
> 2. Please include some rational in the commit message. What prototype or
>    datatype is missing that requires including <linux/types.h>?
Done.
>> From f9881e554deafad1301d00e4539a02fe63078c00 Mon Sep 17 00:00:00 2001
>> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
>> Date: Fri, 16 Apr 2010 09:38:32 +0400
>> Subject: [PATCH 2/2] Add --with-firmware-path configure option
> 
> Again, please include some rational in the commit message. What problem is
> this solving?
Done.
>> @@ -97,6 +94,11 @@ int main(int argc, char **argv)
>>  unsigned int i;
>>  int rc = 0;
>>  
>> +	for (i = 0; i < ARRAY_SIZE(searchpath); i++) {
>> +		printf("Path %s\n", searchpath[i]);
>> +	}
>> +	return 0;
>> +
> 
> I'm pretty sure the program stops doing anything useful with this hunk. :)
I really should stop posting patches after midnight. I hope that the 
attached version is better.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-hid2hci-include-linux-types.h-for-__u32.patch --]
[-- Type: text/x-patch; name="0001-hid2hci-include-linux-types.h-for-__u32.patch", Size: 798 bytes --]

From 6d2d2d4d2c665539c140bbf471c8d1e30baf508b Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 00:21:02 +0400
Subject: [PATCH 1/2] hid2hci: include linux/types.h for __u32

__u32 in linux/hiddev.h is undefined on my laptop (linux-headers-2.6.28.5).
Probably, this is a bug in kernel headers or in my distro but this would not hurt.
---
 extras/hid2hci/hid2hci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/extras/hid2hci/hid2hci.c b/extras/hid2hci/hid2hci.c
index 0d0a022..839c4fb 100644
--- a/extras/hid2hci/hid2hci.c
+++ b/extras/hid2hci/hid2hci.c
@@ -28,6 +28,7 @@
 #include <string.h>
 #include <getopt.h>
 #include <sys/ioctl.h>
+#include <linux/types.h>
 #include <linux/hiddev.h>
 #include <usb.h>
 
-- 
1.7.0.5



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-with-firmware-path-configure-option.patch --]
[-- Type: text/x-patch; name="0002-Add-with-firmware-path-configure-option.patch", Size: 2105 bytes --]

From c810304553d585bc71ffd5633c3cd9241fd2ce15 Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 09:38:32 +0400
Subject: [PATCH 2/2] Add --with-firmware-path configure option

In NixOS we need to use non-standard firmware path: we have no /lib.
---
 configure.ac               |   21 +++++++++++++++++++++
 extras/firmware/firmware.c |    5 +----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 492fa02..cce70d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,26 @@ if test "x$enable_extras" = xyes; then
 		fi])
 	AC_SUBST(PCI_DATABASE)
 
+	AC_ARG_WITH(firmware-path,
+		AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
+		   [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
+		[],
+		[with_firmware_path="/lib/fimware/updates:/lib/fimware"]
+		)
+	OLD_IFS=$IFS
+	IFS=:
+	for i in $with_firmware_path
+	do
+		if test "x${FIRMWARE_PATH}" = "x"
+		then
+			FIRMWARE_PATH="\\\"${i}/\\\""
+		else
+			FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
+		fi
+	done
+	IFS=$OLD_IFS
+	AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
+
 	AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
 	AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
 fi
@@ -144,6 +164,7 @@ AC_MSG_RESULT([
 
 	usb.ids:		${USB_DATABASE}
 	pci.ids:		${PCI_DATABASE}
+	firmware path:		${FIRMWARE_PATH}
 
 	xsltproc:		${XSLTPROC}
 	gperf:			${GPERF}
diff --git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
index 92f0918..76593ba 100644
--- a/extras/firmware/firmware.c
+++ b/extras/firmware/firmware.c
@@ -79,10 +79,7 @@ int main(int argc, char **argv)
 		{ "help", no_argument, NULL, 'h' },
 		{}
 	};
-	static const char *searchpath[] = {
-		"/lib/firmware/updates/",
-		"/lib/firmware/"
-	};
+	static const char *searchpath[] = { FIRMWARE_PATH };
 	char fwencpath[UTIL_PATH_SIZE];
 	char misspath[UTIL_PATH_SIZE];
 	char loadpath[UTIL_PATH_SIZE];
-- 
1.7.0.5

Re: two patches

[Yury G. Kudryashov]

Yury G. Kudryashov wrote:
Ignore the previous message. It doesn't compile.

Re: two patches

[Yury G. Kudryashov]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

Yury G. Kudryashov wrote:

> I really should stop posting patches after midnight. I hope that the
> attached version is better.
I should also stop posting "fixed" versions of patches without re-compiling. 
First patch is OK, new version of the second patch is attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Add-with-firmware-path-configure-option.patch --]
[-- Type: text/x-patch; name="0002-Add-with-firmware-path-configure-option.patch", Size: 2690 bytes --]

From 499a8e76e856b1cec3f045407eeb52a5c3d85a18 Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 09:38:32 +0400
Subject: [PATCH 2/2] Add --with-firmware-path configure option

In NixOS we need to use non-standard firmware path: we have no /lib.
---
 Makefile.am                |    1 +
 configure.ac               |   21 +++++++++++++++++++++
 extras/firmware/firmware.c |    5 +----
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 68a68d9..3b628a4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -194,6 +194,7 @@ dist_udevrules_DATA += \
 # ------------------------------------------------------------------------------
 extras_firmware_firmware_SOURCES = extras/firmware/firmware.c
 extras_firmware_firmware_LDADD = libudev/libudev-private.la
+extras_firmware_firmware_CPPFLAGS = $(AM_CPPFLAGS) -DFIRMWARE_PATH="$(FIRMWARE_PATH)"
 dist_udevrules_DATA += extras/firmware/50-firmware.rules
 libexec_PROGRAMS = extras/firmware/firmware
 
diff --git a/configure.ac b/configure.ac
index 492fa02..cce70d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,26 @@ if test "x$enable_extras" = xyes; then
 		fi])
 	AC_SUBST(PCI_DATABASE)
 
+	AC_ARG_WITH(firmware-path,
+		AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
+		   [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
+		[],
+		[with_firmware_path="/lib/fimware/updates:/lib/fimware"]
+		)
+	OLD_IFS=$IFS
+	IFS=:
+	for i in $with_firmware_path
+	do
+		if test "x${FIRMWARE_PATH}" = "x"
+		then
+			FIRMWARE_PATH="\\\"${i}/\\\""
+		else
+			FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
+		fi
+	done
+	IFS=$OLD_IFS
+	AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
+
 	AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
 	AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
 fi
@@ -144,6 +164,7 @@ AC_MSG_RESULT([
 
 	usb.ids:		${USB_DATABASE}
 	pci.ids:		${PCI_DATABASE}
+	firmware path:		${FIRMWARE_PATH}
 
 	xsltproc:		${XSLTPROC}
 	gperf:			${GPERF}
diff --git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
index 92f0918..76593ba 100644
--- a/extras/firmware/firmware.c
+++ b/extras/firmware/firmware.c
@@ -79,10 +79,7 @@ int main(int argc, char **argv)
 		{ "help", no_argument, NULL, 'h' },
 		{}
 	};
-	static const char *searchpath[] = {
-		"/lib/firmware/updates/",
-		"/lib/firmware/"
-	};
+	static const char *searchpath[] = { FIRMWARE_PATH };
 	char fwencpath[UTIL_PATH_SIZE];
 	char misspath[UTIL_PATH_SIZE];
 	char loadpath[UTIL_PATH_SIZE];
-- 
1.7.0.5

Re: two patches

[Dan Nicholson]

On Fri, Apr 16, 2010 at 10:52:44PM +0400, Yury G. Kudryashov wrote:
> Dan Nicholson wrote:
> 
> > On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
> >> Hi!
> >> 
> >> Review the attached patches, please.
> > 
> >> From 5cc42c6f87f690fdf66e29de2a816dab49a119f4 Mon Sep 17 00:00:00 2001
> >> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> >> Date: Fri, 16 Apr 2010 00:21:02 +0400
> >> Subject: [PATCH 1/2] Include linux/types.h
> > 
> > 1. Prefix the subject with "hid2hci: " so it's clear from the shortlog
> > what
> >    this affects.
> > 
> > 2. Please include some rational in the commit message. What prototype or
> >    datatype is missing that requires including <linux/types.h>?
> Done.
> >> From f9881e554deafad1301d00e4539a02fe63078c00 Mon Sep 17 00:00:00 2001
> >> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> >> Date: Fri, 16 Apr 2010 09:38:32 +0400
> >> Subject: [PATCH 2/2] Add --with-firmware-path configure option
> > 
> > Again, please include some rational in the commit message. What problem is
> > this solving?
> Done.
> >> @@ -97,6 +94,11 @@ int main(int argc, char **argv)
> >>  unsigned int i;
> >>  int rc = 0;
> >>  
> >> +	for (i = 0; i < ARRAY_SIZE(searchpath); i++) {
> >> +		printf("Path %s\n", searchpath[i]);
> >> +	}
> >> +	return 0;
> >> +
> > 
> > I'm pretty sure the program stops doing anything useful with this hunk. :)
> I really should stop posting patches after midnight. I hope that the 
> attached version is better.

> From 6d2d2d4d2c665539c140bbf471c8d1e30baf508b Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 00:21:02 +0400
> Subject: [PATCH 1/2] hid2hci: include linux/types.h for __u32
> 
> __u32 in linux/hiddev.h is undefined on my laptop (linux-headers-2.6.28.5).
> Probably, this is a bug in kernel headers or in my distro but this would not hurt.

Looks good. I see you resent the other patch.

--
Dan

Re: two patches

[Dan Nicholson]

On Fri, Apr 16, 2010 at 11:01:31PM +0400, Yury G. Kudryashov wrote:
> Yury G. Kudryashov wrote:
> 
> > I really should stop posting patches after midnight. I hope that the
> > attached version is better.
> I should also stop posting "fixed" versions of patches without re-compiling. 
> First patch is OK, new version of the second patch is attached.

> From 499a8e76e856b1cec3f045407eeb52a5c3d85a18 Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 09:38:32 +0400
> Subject: [PATCH 2/2] Add --with-firmware-path configure option
> 
> In NixOS we need to use non-standard firmware path: we have no /lib.
> ---
>  Makefile.am                |    1 +
>  configure.ac               |   21 +++++++++++++++++++++
>  extras/firmware/firmware.c |    5 +----
>  3 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 68a68d9..3b628a4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -194,6 +194,7 @@ dist_udevrules_DATA += \
>  # ------------------------------------------------------------------------------
>  extras_firmware_firmware_SOURCES = extras/firmware/firmware.c
>  extras_firmware_firmware_LDADD = libudev/libudev-private.la
> +extras_firmware_firmware_CPPFLAGS = $(AM_CPPFLAGS) -DFIRMWARE_PATH="$(FIRMWARE_PATH)"
>  dist_udevrules_DATA += extras/firmware/50-firmware.rules
>  libexec_PROGRAMS = extras/firmware/firmware
>  
> diff --git a/configure.ac b/configure.ac
> index 492fa02..cce70d1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -82,6 +82,26 @@ if test "x$enable_extras" = xyes; then
>  		fi])
>  	AC_SUBST(PCI_DATABASE)
>  
> +	AC_ARG_WITH(firmware-path,
> +		AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
> +		   [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
> +		[],
> +		[with_firmware_path="/lib/fimware/updates:/lib/fimware"]
> +		)
> +	OLD_IFS=$IFS
> +	IFS=:
> +	for i in $with_firmware_path
> +	do
> +		if test "x${FIRMWARE_PATH}" = "x"
> +		then
> +			FIRMWARE_PATH="\\\"${i}/\\\""
> +		else
> +			FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
> +		fi
> +	done
> +	IFS=$OLD_IFS
> +	AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
> +
>  	AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
>  	AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
>  fi
> @@ -144,6 +164,7 @@ AC_MSG_RESULT([
>  
>  	usb.ids:		${USB_DATABASE}
>  	pci.ids:		${PCI_DATABASE}
> +	firmware path:		${FIRMWARE_PATH}
>  
>  	xsltproc:		${XSLTPROC}
>  	gperf:			${GPERF}
> diff --git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
> index 92f0918..76593ba 100644
> --- a/extras/firmware/firmware.c
> +++ b/extras/firmware/firmware.c
> @@ -79,10 +79,7 @@ int main(int argc, char **argv)
>  		{ "help", no_argument, NULL, 'h' },
>  		{}
>  	};
> -	static const char *searchpath[] = {
> -		"/lib/firmware/updates/",
> -		"/lib/firmware/"
> -	};
> +	static const char *searchpath[] = { FIRMWARE_PATH };
>  	char fwencpath[UTIL_PATH_SIZE];
>  	char misspath[UTIL_PATH_SIZE];
>  	char loadpath[UTIL_PATH_SIZE];
> -- 
> 1.7.0.5

Looks good on inspection, although I didn't test it.

--
Dan