All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] multipath-tools/kpartx - fix endianes, undefined behaviour, cleanup
@ 2005-09-29 12:41 Bastian Blank
  2005-09-29 13:29 ` Christophe Varoqui
  2005-09-29 22:19 ` Christophe Varoqui
  0 siblings, 2 replies; 7+ messages in thread
From: Bastian Blank @ 2005-09-29 12:41 UTC (permalink / raw)
  To: christophe varoqui; +Cc: dm-devel


[-- Attachment #1.1: Type: text/plain, Size: 7954 bytes --]

Hi Christophe

The following patches makes kpartx with dos partitions behave correctly
on powerpc and makes it a little bit easier to read.

You can also pull this changes from
http://137.250.31.225/rsync/git/linux/storage/multipath-tools.git.

Bastian


Use endian.h and byteswap.h.

---
commit 2be146862254c9941cb0754fadd45cb34dd05694
tree 1f6997be9d4b30258d6abfa25aefb2106b61ff96
parent f67cb7d39e9f0a0046d97af3bf78e8c4be2e68bf
author Bastian Blank <waldi@debian.org> Thu, 29 Sep 2005 12:55:36 +0000
committer Bastian Blank <waldi@debian.org> Thu, 29 Sep 2005 12:55:36 +0000

 kpartx/byteorder.h |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/kpartx/byteorder.h b/kpartx/byteorder.h
--- a/kpartx/byteorder.h
+++ b/kpartx/byteorder.h
@@ -1,15 +1,19 @@
 #ifndef BYTEORDER_H_INCLUDED
 #define BYTEORDER_H_INCLUDED
 
-#if defined (__s390__) || defined (__s390x__)
-#define le32_to_cpu(x)	( \
-		(*(((unsigned char *) &(x)))) + \
-		(*(((unsigned char *) &(x))+1) << 8) + \
-		(*(((unsigned char *) &(x))+2) << 16) + \
-		(*(((unsigned char *) &(x))+3) << 24) \
-	)
+#ifdef __linux__
+#  include <endian.h>
+#  include <byteswap.h>
 #else
-#define le32_to_cpu(x)	(x)
+#  error unsupported
+#endif
+
+#if BYTE_ORDER == LITTLE_ENDIAN
+#  define le32_to_cpu(x) (x)
+#elif BYTE_ORDER == BIG_ENDIAN
+#  define le32_to_cpu(x) bswap_32(x)
+#else
+#  error unsupported
 #endif
 
 #endif				/* BYTEORDER_H_INCLUDED */


\f
!-------------------------------------------------------------flip-


Don't use unalligned pointers, always copy the structures to a properly
alligned piece of memory. This triggered undefined behaviour.

---
commit 6399c88650be9835b90bad91a7b6268db8db3303
tree aa0fb4438812f0a116d4e7a7db092bfd45edfcf3
parent 2be146862254c9941cb0754fadd45cb34dd05694
author Bastian Blank <waldi@debian.org> Thu, 29 Sep 2005 13:01:45 +0000
committer Bastian Blank <waldi@debian.org> Thu, 29 Sep 2005 13:01:45 +0000

 kpartx/dos.c |   43 ++++++++++++++++++++-----------------------
 1 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/kpartx/dos.c b/kpartx/dos.c
--- a/kpartx/dos.c
+++ b/kpartx/dos.c
@@ -1,6 +1,7 @@
 #include "kpartx.h"
 #include "byteorder.h"
 #include <stdio.h>
+#include <string.h>
 #include "dos.h"
 
 static int
@@ -12,7 +13,7 @@ static int
 read_extended_partition(int fd, struct partition *ep,
 			struct slice *sp, int ns)
 {
-	struct partition *p;
+	struct partition p;
 	unsigned long start, here;
 	unsigned char *bp;
 	int loopct = 0;
@@ -33,14 +34,13 @@ read_extended_partition(int fd, struct p
 		if (bp[510] != 0x55 || bp[511] != 0xaa)
 			return n;
 
-		p = (struct partition *) (bp + 0x1be);
-
-		for (i=0; i<2; i++, p++) {
-			if (p->nr_sects == 0 || is_extended(p->sys_type))
+		for (i=0; i<2; i++) {
+			memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
+			if (p.nr_sects == 0 || is_extended(p.sys_type))
 				continue;
 			if (n < ns) {
-				sp[n].start = here + le32_to_cpu(p->start_sect);
-				sp[n].size = le32_to_cpu(p->nr_sects);
+				sp[n].start = here + le32_to_cpu(p.start_sect);
+				sp[n].size = le32_to_cpu(p.nr_sects);
 				n++;
 			} else {
 				fprintf(stderr,
@@ -50,10 +50,10 @@ read_extended_partition(int fd, struct p
 			loopct = 0;
 		}
 
-		p -= 2;
-		for (i=0; i<2; i++, p++) {
-			if(p->nr_sects != 0 && is_extended(p->sys_type)) {
-				here = start + le32_to_cpu(p->start_sect);
+		for (i=0; i<2; i++) {
+			memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
+			if(p.nr_sects != 0 && is_extended(p.sys_type)) {
+				here = start + le32_to_cpu(p.start_sect);
 				moretodo = 1;
 				break;
 			}
@@ -69,7 +69,7 @@ is_gpt(int type) {
 
 int
 read_dos_pt(int fd, struct slice all, struct slice *sp, int ns) {
-	struct partition *p;
+	struct partition p;
 	unsigned long offset = all.start;
 	int i, n=0;
 	unsigned char *bp;
@@ -81,32 +81,29 @@ read_dos_pt(int fd, struct slice all, st
 	if (bp[510] != 0x55 || bp[511] != 0xaa)
 		return -1;
 
-	p = (struct partition *) (bp + 0x1be);
 	for (i=0; i<4; i++) {
-		if (is_gpt(p->sys_type)) {
+		memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
+		if (is_gpt(p.sys_type)) {
 			return 0;
 		}
-		p++;
 	}
-	p = (struct partition *) (bp + 0x1be);
 	for (i=0; i<4; i++) {
+		memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
 		/* always add, even if zero length */
 		if (n < ns) {
-			sp[n].start =  le32_to_cpu(p->start_sect);
-			sp[n].size = le32_to_cpu(p->nr_sects);
+			sp[n].start =  le32_to_cpu(p.start_sect);
+			sp[n].size = le32_to_cpu(p.nr_sects);
 			n++;
 		} else {
 			fprintf(stderr,
 				"dos_partition: too many slices\n");
 			break;
 		}
-		p++;
 	}
-	p = (struct partition *) (bp + 0x1be);
 	for (i=0; i<4; i++) {
-		if (is_extended(p->sys_type))
-			n += read_extended_partition(fd, p, sp+n, ns-n);
-		p++;
+		memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
+		if (is_extended(p.sys_type))
+			n += read_extended_partition(fd, &p, sp+n, ns-n);
 	}
 	return n;
 }


\f
!-------------------------------------------------------------flip-


Only use one loop to read all data.

---
commit 1d586d3a1a998b64fa27e61777478d8735d4152a
tree 4553a5ad1ef3367600b00abded4e521ce7629088
parent 6399c88650be9835b90bad91a7b6268db8db3303
author Bastian Blank <waldi@debian.org> Thu, 29 Sep 2005 13:06:34 +0000
committer Bastian Blank <waldi@debian.org> Thu, 29 Sep 2005 13:06:34 +0000

 kpartx/dos.c |   37 ++++++++++++++-----------------------
 1 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/kpartx/dos.c b/kpartx/dos.c
--- a/kpartx/dos.c
+++ b/kpartx/dos.c
@@ -36,8 +36,14 @@ read_extended_partition(int fd, struct p
 
 		for (i=0; i<2; i++) {
 			memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
-			if (p.nr_sects == 0 || is_extended(p.sys_type))
-				continue;
+			if (is_extended(p.sys_type)) {
+				if (p.nr_sects) {
+					here = start + le32_to_cpu(p.start_sect);
+					moretodo = 1;
+				}
+				else
+					continue;
+			}
 			if (n < ns) {
 				sp[n].start = here + le32_to_cpu(p.start_sect);
 				sp[n].size = le32_to_cpu(p.nr_sects);
@@ -49,15 +55,6 @@ read_extended_partition(int fd, struct p
 			}
 			loopct = 0;
 		}
-
-		for (i=0; i<2; i++) {
-			memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
-			if(p.nr_sects != 0 && is_extended(p.sys_type)) {
-				here = start + le32_to_cpu(p.start_sect);
-				moretodo = 1;
-				break;
-			}
-		}
 	}
 	return n;
 }
@@ -71,7 +68,7 @@ int
 read_dos_pt(int fd, struct slice all, struct slice *sp, int ns) {
 	struct partition p;
 	unsigned long offset = all.start;
-	int i, n=0;
+	int i, n=4;
 	unsigned char *bp;
 
 	bp = (unsigned char *)getblock(fd, offset);
@@ -83,25 +80,19 @@ read_dos_pt(int fd, struct slice all, st
 
 	for (i=0; i<4; i++) {
 		memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
-		if (is_gpt(p.sys_type)) {
-			return 0;
-		}
 	}
 	for (i=0; i<4; i++) {
 		memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
-		/* always add, even if zero length */
-		if (n < ns) {
-			sp[n].start =  le32_to_cpu(p.start_sect);
-			sp[n].size = le32_to_cpu(p.nr_sects);
-			n++;
+		if (is_gpt(p.sys_type))
+			return 0;
+		if (i < ns) {
+			sp[i].start =  le32_to_cpu(p.start_sect);
+			sp[i].size = le32_to_cpu(p.nr_sects);
 		} else {
 			fprintf(stderr,
 				"dos_partition: too many slices\n");
 			break;
 		}
-	}
-	for (i=0; i<4; i++) {
-		memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
 		if (is_extended(p.sys_type))
 			n += read_extended_partition(fd, &p, sp+n, ns-n);
 	}


\f
!-------------------------------------------------------------flip-


-- 
Women professionals do tend to over-compensate.
		-- Dr. Elizabeth Dehaver, "Where No Man Has Gone Before",
		   stardate 1312.9.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2005-09-29 22:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-29 12:41 [PATCH] multipath-tools/kpartx - fix endianes, undefined behaviour, cleanup Bastian Blank
2005-09-29 13:29 ` Christophe Varoqui
2005-09-29 14:05   ` Bastian Blank
2005-09-29 14:13     ` Christophe Varoqui
2005-09-29 15:09       ` Bastian Blank
2005-09-29 15:21         ` Christophe Varoqui
2005-09-29 22:19 ` Christophe Varoqui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.