Patchwork [1/1] Limit the kbdrate application to x86, mips and sparc.

login
register
mail settings
Submitter jackie huang
Date June 24, 2013, 6:27 a.m.
Message ID <d520d4fc66725801e83d4719e6c71f9015d8c9b0.1371807568.git.jackie.huang@windriver.com>
Download mbox | patch
Permalink /patch/52257/
State New
Headers show

Comments

jackie huang - June 24, 2013, 6:27 a.m.
From: Jackie Huang <jackie.huang@windriver.com>

The code relies on hardware specific memory locations to access
and modify the keyboard repeat rate.  It also requires read/write
access to /dev/port which doesn't exist on every architecture's
root fs.  The defect was raised for Qemu PowerPC but it also fails on
ARM.  The keyboard emulation in qemuppc is for an ADB (Apple Desktop Bus)
device and not compatible with an Intel driver.  There's also no
indication in the documentation that the code should work on
anything other than Intel architecture but it also works on MIPS.

Signed-off-by: Dennis Hall <dennis.hall@windriver.com>
Signed-off-by: Jackie Huang <jackie.huang@windriver.com>
---
 .../0001-Limit-kbdrate-to-x86-mips-and-sparc.patch |   48 ++++++++++++++++++++
 meta/recipes-core/kbd/kbd_1.15.2.bb                |    6 ++-
 2 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100644 meta/recipes-core/kbd/files/0001-Limit-kbdrate-to-x86-mips-and-sparc.patch
Paul Barker - June 24, 2013, 7:23 a.m.
On 24 Jun 2013 07:28, <jackie.huang@windriver.com> wrote:
>
> From: Jackie Huang <jackie.huang@windriver.com>
>
> The code relies on hardware specific memory locations to access
> and modify the keyboard repeat rate.  It also requires read/write
> access to /dev/port which doesn't exist on every architecture's
> root fs.  The defect was raised for Qemu PowerPC but it also fails on
> ARM.  The keyboard emulation in qemuppc is for an ADB (Apple Desktop Bus)
> device and not compatible with an Intel driver.  There's also no
> indication in the documentation that the code should work on
> anything other than Intel architecture but it also works on MIPS.
>

> ++#if !(  defined(__i386__)   \
> ++     || defined(__x86_64__) \
> ++     || defined(__mips__)   \
> ++     || defined(__sparc___))
> ++
> ++      fprintf(stderr,
> ++              "ERROR:  %s should only be used on MIPS, x86 and Sparc
archicture boards\n", basename(argv[0]));
> ++        exit(1);
> ++#endif

Would it be better to detect the target platform in the build system and
skip building the kbdrate program for architectures it won't work on rather
than building and installing a known broken program?

> ++      set_progname(argv[0]);
> +       set_progname(argv[0]);

Accidental repeat?

--

Paul Barker

Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk
jackie huang - June 24, 2013, 7:45 a.m.
On 6/24/2013 3:23 PM, Paul Barker wrote:
> On 24 Jun 2013 07:28, <jackie.huang@windriver.com
> <mailto:jackie.huang@windriver.com>> wrote:
>  >
>  > From: Jackie Huang <jackie.huang@windriver.com
> <mailto:jackie.huang@windriver.com>>
>  >
>  > The code relies on hardware specific memory locations to access
>  > and modify the keyboard repeat rate.  It also requires read/write
>  > access to /dev/port which doesn't exist on every architecture's
>  > root fs.  The defect was raised for Qemu PowerPC but it also fails on
>  > ARM.  The keyboard emulation in qemuppc is for an ADB (Apple Desktop Bus)
>  > device and not compatible with an Intel driver.  There's also no
>  > indication in the documentation that the code should work on
>  > anything other than Intel architecture but it also works on MIPS.
>  >
>
>  > ++#if !(  defined(__i386__)   \
>  > ++     || defined(__x86_64__) \
>  > ++     || defined(__mips__)   \
>  > ++     || defined(__sparc___))
>  > ++
>  > ++      fprintf(stderr,
>  > ++              "ERROR:  %s should only be used on MIPS, x86 and
> Sparc archicture boards\n", basename(argv[0]));
>  > ++        exit(1);
>  > ++#endif
>
> Would it be better to detect the target platform in the build system and
> skip building the kbdrate program for architectures it won't work on
> rather than building and installing a known broken program?

Yeah, it should be better, I will check if we can do that.

>
>  > ++      set_progname(argv[0]);
>  > +       set_progname(argv[0]);
>
> Accidental repeat?

Indeed, thanks for pointing this out.

Thanks,
Jackie

>
> --
>
> Paul Barker
>
> Email: paul@paulbarker.me.uk <mailto:paul@paulbarker.me.uk>
> http://www.paulbarker.me.uk
>
Saul Wold - June 24, 2013, 3:18 p.m.
On 06/24/2013 12:45 AM, jhuang0 wrote:
>
>
> On 6/24/2013 3:23 PM, Paul Barker wrote:
>> On 24 Jun 2013 07:28, <jackie.huang@windriver.com
>> <mailto:jackie.huang@windriver.com>> wrote:
>>  >
>>  > From: Jackie Huang <jackie.huang@windriver.com
>> <mailto:jackie.huang@windriver.com>>
>>  >
>>  > The code relies on hardware specific memory locations to access
>>  > and modify the keyboard repeat rate.  It also requires read/write
>>  > access to /dev/port which doesn't exist on every architecture's
>>  > root fs.  The defect was raised for Qemu PowerPC but it also fails on
>>  > ARM.  The keyboard emulation in qemuppc is for an ADB (Apple
>> Desktop Bus)
>>  > device and not compatible with an Intel driver.  There's also no
>>  > indication in the documentation that the code should work on
>>  > anything other than Intel architecture but it also works on MIPS.
>>  >
>>
>>  > ++#if !(  defined(__i386__)   \
>>  > ++     || defined(__x86_64__) \
>>  > ++     || defined(__mips__)   \
>>  > ++     || defined(__sparc___))
>>  > ++
>>  > ++      fprintf(stderr,
>>  > ++              "ERROR:  %s should only be used on MIPS, x86 and
>> Sparc archicture boards\n", basename(argv[0]));
>>  > ++        exit(1);
>>  > ++#endif
>>
>> Would it be better to detect the target platform in the build system and
>> skip building the kbdrate program for architectures it won't work on
>> rather than building and installing a known broken program?
>
> Yeah, it should be better, I will check if we can do that.
>
Use COMPATIBLE_MACHINE

Sau!

>>
>>  > ++      set_progname(argv[0]);
>>  > +       set_progname(argv[0]);
>>
>> Accidental repeat?
>
> Indeed, thanks for pointing this out.
>
> Thanks,
> Jackie
>
>>
>> --
>>
>> Paul Barker
>>
>> Email: paul@paulbarker.me.uk <mailto:paul@paulbarker.me.uk>
>> http://www.paulbarker.me.uk
>>
>
Richard Purdie - June 24, 2013, 3:23 p.m.
On Mon, 2013-06-24 at 08:18 -0700, Saul Wold wrote:
> On 06/24/2013 12:45 AM, jhuang0 wrote:
> >
> >
> > On 6/24/2013 3:23 PM, Paul Barker wrote:
> >> On 24 Jun 2013 07:28, <jackie.huang@windriver.com
> >> <mailto:jackie.huang@windriver.com>> wrote:
> >>  >
> >>  > From: Jackie Huang <jackie.huang@windriver.com
> >> <mailto:jackie.huang@windriver.com>>
> >>  >
> >>  > The code relies on hardware specific memory locations to access
> >>  > and modify the keyboard repeat rate.  It also requires read/write
> >>  > access to /dev/port which doesn't exist on every architecture's
> >>  > root fs.  The defect was raised for Qemu PowerPC but it also fails on
> >>  > ARM.  The keyboard emulation in qemuppc is for an ADB (Apple
> >> Desktop Bus)
> >>  > device and not compatible with an Intel driver.  There's also no
> >>  > indication in the documentation that the code should work on
> >>  > anything other than Intel architecture but it also works on MIPS.
> >>  >
> >>
> >>  > ++#if !(  defined(__i386__)   \
> >>  > ++     || defined(__x86_64__) \
> >>  > ++     || defined(__mips__)   \
> >>  > ++     || defined(__sparc___))
> >>  > ++
> >>  > ++      fprintf(stderr,
> >>  > ++              "ERROR:  %s should only be used on MIPS, x86 and
> >> Sparc archicture boards\n", basename(argv[0]));
> >>  > ++        exit(1);
> >>  > ++#endif
> >>
> >> Would it be better to detect the target platform in the build system and
> >> skip building the kbdrate program for architectures it won't work on
> >> rather than building and installing a known broken program?
> >
> > Yeah, it should be better, I will check if we can do that.
> >
> Use COMPATIBLE_MACHINE

We're after to take out a single binary, not the whole recipe?

Cheers,

Richard
jackie huang - June 25, 2013, 1:19 a.m.
On 6/24/2013 11:23 PM, Richard Purdie wrote:
> On Mon, 2013-06-24 at 08:18 -0700, Saul Wold wrote:
>> On 06/24/2013 12:45 AM, jhuang0 wrote:
>>>
>>>
>>> On 6/24/2013 3:23 PM, Paul Barker wrote:
>>>> On 24 Jun 2013 07:28, <jackie.huang@windriver.com
>>>> <mailto:jackie.huang@windriver.com>> wrote:
>>>>   >
>>>>   > From: Jackie Huang <jackie.huang@windriver.com
>>>> <mailto:jackie.huang@windriver.com>>
>>>>   >
>>>>   > The code relies on hardware specific memory locations to access
>>>>   > and modify the keyboard repeat rate.  It also requires read/write
>>>>   > access to /dev/port which doesn't exist on every architecture's
>>>>   > root fs.  The defect was raised for Qemu PowerPC but it also fails on
>>>>   > ARM.  The keyboard emulation in qemuppc is for an ADB (Apple
>>>> Desktop Bus)
>>>>   > device and not compatible with an Intel driver.  There's also no
>>>>   > indication in the documentation that the code should work on
>>>>   > anything other than Intel architecture but it also works on MIPS.
>>>>   >
>>>>
>>>>   > ++#if !(  defined(__i386__)   \
>>>>   > ++     || defined(__x86_64__) \
>>>>   > ++     || defined(__mips__)   \
>>>>   > ++     || defined(__sparc___))
>>>>   > ++
>>>>   > ++      fprintf(stderr,
>>>>   > ++              "ERROR:  %s should only be used on MIPS, x86 and
>>>> Sparc archicture boards\n", basename(argv[0]));
>>>>   > ++        exit(1);
>>>>   > ++#endif
>>>>
>>>> Would it be better to detect the target platform in the build system and
>>>> skip building the kbdrate program for architectures it won't work on
>>>> rather than building and installing a known broken program?
>>>
>>> Yeah, it should be better, I will check if we can do that.
>>>
>> Use COMPATIBLE_MACHINE
>
> We're after to take out a single binary, not the whole recipe?

Yes, not the whole recipe, kbdrate is just one of the biaries provided 
by the kbd package, so I think it's not appropriate to use 
COMPATIBLE_MACHINE here.

Thanks,
Jackie

>
> Cheers,
>
> Richard
>
>

Patch

diff --git a/meta/recipes-core/kbd/files/0001-Limit-kbdrate-to-x86-mips-and-sparc.patch b/meta/recipes-core/kbd/files/0001-Limit-kbdrate-to-x86-mips-and-sparc.patch
new file mode 100644
index 0000000..0283202
--- /dev/null
+++ b/meta/recipes-core/kbd/files/0001-Limit-kbdrate-to-x86-mips-and-sparc.patch
@@ -0,0 +1,48 @@ 
+From 4a679f16a3728a9323d23a8466d22a984a58c7af Mon Sep 17 00:00:00 2001
+From: Dennis Hall <dennis.hall@windriver.com>
+Date: Fri, 31 Aug 2012 10:15:13 -0400
+Subject: [PATCH] Limit kbdrate to x86, mips and sparc.
+
+Upstream-Status: Pending
+
+The application will exit with an error if it runs on any other architecture.
+
+The code relies on hardware specific memory locations to access
+and modify the keyboard repeat rate.  It also requires read/write
+access to /dev/port which doesn't exist on every architecture's
+root fs.  The CQ was raised for Qemu PowerPC but it also fails on
+ARM.  The keyboard emulation in qemuppc is for an ADB (Apple Desktop Bus)
+device and not compatible with an Intel driver.  There's also no
+indication in the documentation that the code should work on
+anything other than Intel architecture but it also works on MIPS32.
+MIPS64 is not supported at this time so it was not tested.
+
+Signed-off-by: Dennis Hall <dennis.hall@windriver.com>
+---
+ src/kbdrate.c |   10 ++++++++++
+ 1 files changed, 10 insertions(+), 0 deletions(-)
+
+diff --git a/src/kbdrate.c b/src/kbdrate.c
+index 95ff723..9a79774 100644
+--- a/src/kbdrate.c
++++ b/src/kbdrate.c
+@@ -241,6 +241,16 @@ main( int argc, char **argv ) {
+ 	unsigned int i;
+ 	extern char *optarg;
+
++#if !(  defined(__i386__)   \
++     || defined(__x86_64__) \
++     || defined(__mips__)   \
++     || defined(__sparc___))
++
++	fprintf(stderr,
++		"ERROR:  %s should only be used on MIPS, x86 and Sparc archicture boards\n", basename(argv[0]));
++        exit(1);
++#endif
++ 	set_progname(argv[0]);
+ 	set_progname(argv[0]);
+
+ 	setlocale(LC_ALL, "");
+--
+1.7.1
+
diff --git a/meta/recipes-core/kbd/kbd_1.15.2.bb b/meta/recipes-core/kbd/kbd_1.15.2.bb
index 79ea468..2fddf0a 100644
--- a/meta/recipes-core/kbd/kbd_1.15.2.bb
+++ b/meta/recipes-core/kbd/kbd_1.15.2.bb
@@ -10,9 +10,11 @@  RREPLACES_${PN} = "console-tools"
 RPROVIDES_${PN} = "console-tools"
 RCONFLICTS_${PN} = "console-tools"
 
-PR = "r4"
+PR = "r5"
 
-SRC_URI="${KERNELORG_MIRROR}/linux/utils/kbd/kbd-1.15.2.tar.bz2"
+SRC_URI="${KERNELORG_MIRROR}/linux/utils/kbd/kbd-1.15.2.tar.bz2 \
+         file://0001-Limit-kbdrate-to-x86-mips-and-sparc.patch \
+"
 SRC_URI[md5sum] = "e850eb91e4d3b94b194efe8e953204c5"
 SRC_URI[sha256sum] = "b3602d191eef7a6a8317fc3cd231efa40a89ac235dce57a77cac825a2a21eba6"