Re: misaligned memory access in cmdlinepart.c

["Timofei V. Bondarenko" <timm@ipi.ac.ru>]

Jörn Engel wrote:
> On Mon, 6 June 2005 18:01:16 +0400, Timofei V. Bondarenko wrote:
> 
>>Index: cmdlinepart.c
>>===================================================================
>>RCS file: /home/cvs/mtd/drivers/mtd/cmdlinepart.c,v
>>retrieving revision 1.17
>>diff -u -p -r1.17 cmdlinepart.c
>>--- cmdlinepart.c	26 Nov 2004 11:18:47 -0000	1.17
>>+++ cmdlinepart.c	6 Jun 2005 13:57:08 -0000
>>@@ -234,12 +234,14 @@ static int mtdpart_setup_real(char *s)
>> 		 * parse one mtd. have it reserve memory for the
>> 		 * struct cmdline_mtd_partition and the mtd-id string.
>> 		 */
>>+#define THIS_MTD_ALIGN_CONST sizeof(void*)
> 
> Not sure if this macro is worth the pain.  If it is, you should
> declare it outside the function.  Just above is what most people do.

Ok.. That's matter of taste. I'm rather like to complicate things.

>>+		/* align this_mtd */
>>+                this_mtd = (struct cmdline_mtd_partition *)
>>+                    ALIGN((unsigned long)this_mtd, THIS_MTD_ALIGN_CONST);
>>+		/* enter results */
> 
> 
> Gcc allows arithmetic with (void*) exactly for code like this:
> 		this_mtd = ALIGN((void*)this_mtd, THIS_MTD_ALIGN_CONST);
> or:
> 		this_mtd = ALIGN((void*)this_mtd, sizeof(void*));
> 
> You should retest it, though.

error: invalid operands to binary &

Then we've to cast it from integer to a pointer type.

--
	Timofei.


Index: cmdlinepart.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/cmdlinepart.c,v
retrieving revision 1.17
diff -u -p -r1.17 cmdlinepart.c
--- cmdlinepart.c	26 Nov 2004 11:18:47 -0000	1.17
+++ cmdlinepart.c	6 Jun 2005 14:33:59 -0000
@@ -239,7 +239,8 @@ static int mtdpart_setup_real(char *s)
  				&num_parts,	/* out: number of parts */
  				0,		/* first partition */
  				(unsigned char**)&this_mtd, /* out: extra mem */
-				mtd_id_len + 1 + sizeof(*this_mtd));
+				mtd_id_len + 1 + sizeof(*this_mtd) +
+                                sizeof(void*)-1 /*alignment*/);
  		if(!parts)
  		{
  			/*
@@ -252,7 +253,10 @@ static int mtdpart_setup_real(char *s)
  			 return 0;
  		 }

-		/* enter results */	
+		/* align this_mtd */
+                this_mtd = (struct cmdline_mtd_partition *)
+                    ALIGN((unsigned long)this_mtd, sizeof(void*));
+		/* enter results */
  		this_mtd->parts = parts;
  		this_mtd->num_parts = num_parts;
  		this_mtd->mtd_id = (char*)(this_mtd + 1);