util-linux-native: fix qsort_r for CentOS 5.10

The qsort_r() was added to glibc in version 2.8, so there is no qsort_r() on the host like CentOS 5.x, use qsort() to fix it since they are nearly identical.

Signed-off-by Robert Yang <[email protected]>

---
 .../util-linux/util-linux-native-qsort.patch       |   34 ++++++++++++++++++++
 meta/recipes-core/util-linux/util-linux_2.24.1.bb  |    4 ++-
 2 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file +18 -0

new file mode 100644
index 0000000..1707683
@@ -0,0 +1,34 @@
+From f220d809be1baa654503bf6ff52f3630b0d7015c Mon Sep 17 00:00:00 2001
+From: Robert Yang <[email protected]>
+Date: Wed, 26 Mar 2014 01:30:29 +0000
+Subject: [PATCH] sun.c: use qsort() to instead of qsort_r()
+
+qsort_r() was added to glibc in version 2.8, so there is no qsort_r() on
+the host like CentOS 5.x.
+
+Upstream-Status: Inappropriate [Other]
+
+Signed-off-by: Robert Yang <[email protected]>
+---
+ libfdisk/src/sun.c | 5 ++---
+ 1 file changed, 2 insertions(+), 3 deletions(-)
+
+diff --git a/libfdisk/src/sun.c b/libfdisk/src/sun.c
+index e73c701..f7899ec 100644
+--- a/libfdisk/src/sun.c

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file +6 -0

+
+     if (array[0] == -1) {
+       fdisk_info(cxt, _("No partitions defined."));
+--
+1.8.2.1
+

meta/recipes-core/util-linux/util-linux_2.24.1.bb +3 -2

index aa98b65..ab80ab6 100644
@@ -4,7 +4,9 @@ require util-linux.inc
 # To support older hosts, we need to patch and/or revert
 # some upstream changes.  Only do this for native packages.
 OLDHOST = ""
-OLDHOST_class-native = "file://util-linux-native.patch"
+OLDHOST_class-native = "file://util-linux-native.patch \
+                        file://util-linux-native-qsort.patch \
+                       "
 
 SRC_URI += "file://util-linux-ng-replace-siginterrupt.patch \
             file://util-linux-ng-2.16-mount_lock_path.patch \
--

1.7.10.4

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Quoted 42 lines · Robert Yang

The qsort_r() was added to glibc in version 2.8, so there is no qsort_r() on the host like CentOS 5.x, use qsort() to fix it since they are nearly identical.

Signed-off-by Robert Yang <[email protected]>

---
  .../util-linux/util-linux-native-qsort.patch       |   34 ++++++++++++++++++++
  meta/recipes-core/util-linux/util-linux_2.24.1.bb  |    4 ++-
  2 files changed, 37 insertions(+), 1 deletion(-)

create mode 100644 meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file +25 -0

new file mode 100644
index 0000000..1707683
@@ -0,0 +1,34 @@
+From f220d809be1baa654503bf6ff52f3630b0d7015c Mon Sep 17 00:00:00 2001
+From: Robert Yang <[email protected]>
+Date: Wed, 26 Mar 2014 01:30:29 +0000
+Subject: [PATCH] sun.c: use qsort() to instead of qsort_r()
+
+qsort_r() was added to glibc in version 2.8, so there is no qsort_r() on
+the host like CentOS 5.x.
+
+Upstream-Status: Inappropriate [Other]
+
+Signed-off-by: Robert Yang <[email protected]>
+---
+ libfdisk/src/sun.c | 5 ++---
+ 1 file changed, 2 insertions(+), 3 deletions(-)
+
+diff --git a/libfdisk/src/sun.c b/libfdisk/src/sun.c
+index e73c701..f7899ec 100644
+--- a/libfdisk/src/sun.c
++++ b/libfdisk/src/sun.c
+@@ -427,9 +427,8 @@ static int sun_verify_disklabel(struct fdisk_context *cxt)
+         else
+             array[i] = -1;
+     }
+-    qsort_r(array,ARRAY_SIZE(array),sizeof(array[0]),
+-        (int (*)(const void *,const void *,void *)) verify_sun_cmp,

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch line 28

Quoted 3 lines

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file +3 -0

+-        verify_sun_starts);
++    qsort(array,ARRAY_SIZE(array),sizeof(array[0]),
++        (int (*)(const void *,const void *)) verify_sun_cmp);

I've just been looking at this for building util-linux on top of musl (as musl-libc doesn't implement qsort_r)
and my solution was to import a qsort_r implementation from ccl
(https://ccl.googlecode.com/svn/trunk/qsort_r.c).

Are you sure this solution works? verify_sun_cmp takes 3 parameters not 2 and it uses the 3rd parameter (data). From reading this I'd imagine a segfault is likely to occur in verify_sun_cmp if qsort is used instead of qsort_r.

Quoted 24 lines

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file +6 -0

+
+     if (array[0] == -1) {
+       fdisk_info(cxt, _("No partitions defined."));
+--
+1.8.2.1
+

meta/recipes-core/util-linux/util-linux_2.24.1.bb +3 -2

index aa98b65..ab80ab6 100644
@@ -4,7 +4,9 @@ require util-linux.inc
  # To support older hosts, we need to patch and/or revert
  # some upstream changes.  Only do this for native packages.
  OLDHOST = ""
-OLDHOST_class-native = "file://util-linux-native.patch"
+OLDHOST_class-native = "file://util-linux-native.patch \
+                        file://util-linux-native-qsort.patch \
+                       "
 
  SRC_URI += "file://util-linux-ng-replace-siginterrupt.patch \
              file://util-linux-ng-2.16-mount_lock_path.patch \
--

1.7.10.4

Quoted 5 lines

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Thanks,

Quoted 53 lines · Paul Barker

Quoted 51 lines · Robert Yang

The qsort_r() was added to glibc in version 2.8, so there is no qsort_r() on
the host like CentOS 5.x, use qsort() to fix it since they are nearly identical.

Signed-off-by Robert Yang <[email protected]>

---
  .../util-linux/util-linux-native-qsort.patch       |   34
++++++++++++++++++++
  meta/recipes-core/util-linux/util-linux_2.24.1.bb  |    4 ++-
  2 files changed, 37 insertions(+), 1 deletion(-)

create mode 100644
meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch

diff --git a/meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch b/meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file mode 100644
index 0000000..1707683
--- /dev/null
+++
b/meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file +20 -0

@@ -0,0 +1,34 @@
+From f220d809be1baa654503bf6ff52f3630b0d7015c Mon Sep 17 00:00:00 2001
+From: Robert Yang <[email protected]>
+Date: Wed, 26 Mar 2014 01:30:29 +0000
+Subject: [PATCH] sun.c: use qsort() to instead of qsort_r()
+
+qsort_r() was added to glibc in version 2.8, so there is no qsort_r() on
+the host like CentOS 5.x.
+
+Upstream-Status: Inappropriate [Other]
+
+Signed-off-by: Robert Yang <[email protected]>
+---
+ libfdisk/src/sun.c | 5 ++---
+ 1 file changed, 2 insertions(+), 3 deletions(-)
+
+diff --git a/libfdisk/src/sun.c b/libfdisk/src/sun.c
+index e73c701..f7899ec 100644
+--- a/libfdisk/src/sun.c
++++ b/libfdisk/src/sun.c
+@@ -427,9 +427,8 @@ static int sun_verify_disklabel(struct fdisk_context

*cxt)

meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch new file +6 -0

+         else
+             array[i] = -1;
+     }
+-    qsort_r(array,ARRAY_SIZE(array),sizeof(array[0]),
+-        (int (*)(const void *,const void *,void *)) verify_sun_cmp,
+-        verify_sun_starts);

Quoted 9 lines

Quoted 2 lines

++ qsort(array,ARRAY_SIZE(array),sizeof(array[0]),
++ (int (*)(const void *,const void *)) verify_sun_cmp);

I've just been looking at this for building util-linux on top of musl (as musl-libc doesn't implement qsort_r)
and my solution was to import a qsort_r implementation from ccl
(https://ccl.googlecode.com/svn/trunk/qsort_r.c).

Maybe we can do it in YP 1.7.

Quoted 5 lines

Are you sure this solution works? verify_sun_cmp takes 3 parameters not 2 and it uses the 3rd parameter (data). From reading this I'd imagine a segfault is likely to occur in verify_sun_cmp if qsort is used instead of qsort_r.

I think it works well since there is a similar patch before we upgrade the util-linux-native, I will verify later.

The more I look at it the more I don't like this patch. It's probably a very rarely used code path but it could blow up if it's called. C provides no guarantees that calling a 3-argument function with only 2 arguments will work. Depending on calling convention it could easily result in stack corruption on some platforms.

I'd suggest we try reverting the relevant bits of the upstream change
from qsort to
qsort_r: http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=c69bbca9c1f6645097bd20fe3a21f5a99a2a0698

I think it should just be the first 3 patch hunks in that commit.

Quoted 10 lines · Paul Barker

The more I look at it the more I don't like this patch. It's probably a very rarely used code path but it could blow up if it's called. C provides no guarantees that calling a 3-argument function with only 2 arguments will work. Depending on calling convention it could easily result in stack corruption on some platforms.

I'd suggest we try reverting the relevant bits of the upstream change
from qsort to
qsort_r: http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=c69bbca9c1f6645097bd20fe3a21f5a99a2a0698

Quoted 1 line

I think it should just be the first 3 patch hunks in that commit.

It seems vanishingly unlikely that anybody is seriously going to try to use fdisk from util-linux-native to manipulate Sun partition tables and, that being the case, it's presumably going to be quite hard to test any such change. Maybe we should just turn off support for these fringe partition table types altogether.

Quoted 13 lines · Phil Blundell

Quoted 11 lines · Paul Barker

The more I look at it the more I don't like this patch. It's probably a very rarely used code path but it could blow up if it's called. C provides no guarantees that calling a 3-argument function with only 2 arguments will work. Depending on calling convention it could easily result in stack corruption on some platforms.

I'd suggest we try reverting the relevant bits of the upstream change
from qsort to
qsort_r: http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=c69bbca9c1f6645097bd20fe3a21f5a99a2a0698

I think it should just be the first 3 patch hunks in that commit.

Quoted 6 lines

It seems vanishingly unlikely that anybody is seriously going to try to use fdisk from util-linux-native to manipulate Sun partition tables and, that being the case, it's presumably going to be quite hard to test any such change. Maybe we should just turn off support for these fringe partition table types altogether.

That may be a much, much easier fix.

I've reported this to the upstream mailing list anyway as there really should be an autoconf check for qsort_r.

Thanks,

Quoted 26 lines · Paul Barker

Quoted 23 lines · Phil Blundell

Quoted 7 lines · Paul Barker

The more I look at it the more I don't like this patch. It's probably a very rarely used code path but it could blow up if it's called. C provides no guarantees that calling a 3-argument function with only 2 arguments will work. Depending on calling convention it could easily result in stack corruption on some platforms.

Quoted 6 lines

I'd suggest we try reverting the relevant bits of the upstream change from qsort to
qsort_r:
http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=c69bbca9c1f6645097bd20fe3a21f5a99a2a0698

I think it should just be the first 3 patch hunks in that commit.

It seems vanishingly unlikely that anybody is seriously going to try to use fdisk from util-linux-native to manipulate Sun partition tables and, that being the case, it's presumably going to be quite hard to test any such change. Maybe we should just turn off support for these fringe partition table types altogether.

Quoted 1 line

That may be a much, much easier fix.

Thank you very much, I will send a patch later:-)

Quoted 32 lines · Robert Yang

Quoted 27 lines · Paul Barker

Quoted 14 lines · Phil Blundell

Quoted 12 lines · Paul Barker

The more I look at it the more I don't like this patch. It's probably a very rarely used code path but it could blow up if it's called. C provides no guarantees that calling a 3-argument function with only 2 arguments will work. Depending on calling convention it could easily result in stack corruption on some platforms.

I'd suggest we try reverting the relevant bits of the upstream change from qsort to
qsort_r:
http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=c69bbca9c1f6645097bd20fe3a21f5a99a2a0698

Quoted 9 lines

Quoted 1 line

I think it should just be the first 3 patch hunks in that commit.

It seems vanishingly unlikely that anybody is seriously going to try to use fdisk from util-linux-native to manipulate Sun partition tables and, that being the case, it's presumably going to be quite hard to test any such change. Maybe we should just turn off support for these fringe partition table types altogether.

That may be a much, much easier fix.

Quoted 2 lines

Thank you very much, I will send a patch later:-)

Sorry to confuse the matter further but it seems Khem has also made a fix for this which removes qsort_r in a more reliable way, along with removing a few other non-posix compliant bits:
http://git.openembedded.org/openembedded-core-contrib/commit/?h=kraj/musl&id=0e0d93d8d5fdcdc9cbcd9da2be1d0193a06d3188

I'm happy with that fix, could you let me know if it works for you Robert?