[patch] fix void* arithmetic

[patch] fix void* arithmetic

[Hollis Blanchard]

Hi Ian, I needed the following patch to avoid lots of these warnings:
	elf.c:238: warning: pointer of type `void *' used in arithmetic

Fix void* arithmetic warnings.
Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

diff -r 030a041bbe90 xen/common/elf.c
--- a/xen/common/elf.c  Tue Aug 29 06:53:58 2006 -0400
+++ b/xen/common/elf.c  Tue Aug 29 15:22:26 2006 -0500
@@ -102,9 +102,9 @@ static unsigned long long xen_guest_nume
 /*
  * Interface to the Xen ELF notes.
  */
-#define ELFNOTE_NAME(_n_)   ((void*)(_n_) + sizeof(*(_n_)))
-#define ELFNOTE_DESC(_n_)   (ELFNOTE_NAME(_n_) + (((_n_)->namesz+3)&~3))
-#define ELFNOTE_NEXT(_n_)   (ELFNOTE_DESC(_n_) + (((_n_)->descsz+3)&~3))
+#define ELFNOTE_NAME(_n_)   ((void*)((char*)(_n_) + sizeof(*(_n_))))
+#define ELFNOTE_DESC(_n_)   (((void*)((char*)ELFNOTE_NAME(_n_) + (((_n_)->namesz+3)&~3))))
+#define ELFNOTE_NEXT(_n_)   (((void*)((char*)ELFNOTE_DESC(_n_) + (((_n_)->descsz+3)&~3))))

 static int is_xen_elfnote_section(const char *image, Elf_Shdr *shdr)
 {
@@ -235,9 +235,9 @@ int parseelfimage(struct domain_setup_in
         shdr = (Elf_Shdr *)(image + ehdr->e_shoff + (h*ehdr->e_shentsize));
         if ( !is_xen_elfnote_section(image, shdr) )
             continue;
-        dsi->__elfnote_section = (void *)image + shdr->sh_offset;
+        dsi->__elfnote_section = (char *)image + shdr->sh_offset;
         dsi->__elfnote_section_end =
-            (void *)image + shdr->sh_offset + shdr->sh_size;
+            (char *)image + shdr->sh_offset + shdr->sh_size;
         break;
     }



-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [patch] fix void* arithmetic

[Aron Griffis]

Hollis Blanchard wrote:  [Tue Aug 29 2006, 04:24:48PM EDT]
> +#define ELFNOTE_NAME(_n_)   ((void*)((char*)(_n_) + sizeof(*(_n_))))
> +#define ELFNOTE_DESC(_n_)   (((void*)((char*)ELFNOTE_NAME(_n_) + (((_n_)->namesz+3)&~3))))
> +#define ELFNOTE_NEXT(_n_)   (((void*)((char*)ELFNOTE_DESC(_n_) + (((_n_)->descsz+3)&~3))))

Too many parens on the latter two?  I think this is the same:

#define ELFNOTE_NAME(_n_)   ((void*)((char*)(_n_) + sizeof(*(_n_))))
#define ELFNOTE_DESC(_n_)   ((void*)((char*)ELFNOTE_NAME(_n_) + (((_n_)->namesz+3)&~3)))
#define ELFNOTE_NEXT(_n_)   ((void*)((char*)ELFNOTE_DESC(_n_) + (((_n_)->descsz+3)&~3)))

Aron

Re: [patch] fix void* arithmetic

[Ian Campbell]

On Tue, 2006-08-29 at 15:24 -0500, Hollis Blanchard wrote:
> Hi Ian, I needed the following patch to avoid lots of these warnings:
> 	elf.c:238: warning: pointer of type `void *' used in arithmetic
> 
> Fix void* arithmetic warnings.
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

Looks like PPC is the only arch using -Wpointer-arith, is there a reason
for that?

Ian.

Re: Re: [patch] fix void* arithmetic

[Hollis Blanchard]

On Tue, 2006-08-29 at 22:06 +0100, Ian Campbell wrote:
> On Tue, 2006-08-29 at 15:24 -0500, Hollis Blanchard wrote:
> > Hi Ian, I needed the following patch to avoid lots of these warnings:
> > 	elf.c:238: warning: pointer of type `void *' used in arithmetic
> > 
> > Fix void* arithmetic warnings.
> > Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> 
> Looks like PPC is the only arch using -Wpointer-arith, is there a reason
> for that?

Is there are reason the other architectures *aren't* using it?

We have some extra warnings enabled because they've helped us in the
past (such as -Wshadow). Given that we're just playing janitor for
everyone else's code though, I think we're about to abandon that one.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: Re: [patch] fix void* arithmetic

[Jeff Garzik]

Hollis Blanchard wrote:
> On Tue, 2006-08-29 at 22:06 +0100, Ian Campbell wrote:
>> On Tue, 2006-08-29 at 15:24 -0500, Hollis Blanchard wrote:
>>> Hi Ian, I needed the following patch to avoid lots of these warnings:
>>> 	elf.c:238: warning: pointer of type `void *' used in arithmetic
>>>
>>> Fix void* arithmetic warnings.
>>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>> Looks like PPC is the only arch using -Wpointer-arith, is there a reason
>> for that?
> 
> Is there are reason the other architectures *aren't* using it?
> 
> We have some extra warnings enabled because they've helped us in the
> past (such as -Wshadow). Given that we're just playing janitor for
> everyone else's code though, I think we're about to abandon that one.

Pointer arith is quite valid on void pointers, when using gcc and most 
other modern compilers.

Point of fact, any Linux kernel-related code REQUIRES that void* arith 
be valid, and not cause warnings.

As we see from your patch, all a cast to char* does is complicate the 
code, for zero gain.

	Jeff

Re: [patch] fix void* arithmetic

[Jan Beulich]

>-#define ELFNOTE_NAME(_n_)   ((void*)(_n_) + sizeof(*(_n_)))
>-#define ELFNOTE_DESC(_n_)   (ELFNOTE_NAME(_n_) + (((_n_)->namesz+3)&~3))
>-#define ELFNOTE_NEXT(_n_)   (ELFNOTE_DESC(_n_) + (((_n_)->descsz+3)&~3))
>+#define ELFNOTE_NAME(_n_)   ((void*)((char*)(_n_) + sizeof(*(_n_))))
>+#define ELFNOTE_DESC(_n_)   (((void*)((char*)ELFNOTE_NAME(_n_) + (((_n_)->namesz+3)&~3))))
>+#define ELFNOTE_NEXT(_n_)   (((void*)((char*)ELFNOTE_DESC(_n_) + (((_n_)->descsz+3)&~3))))

This seems to complicate it a little too much, eg it would seem that this

#define ELFNOTE_NAME(_n_)   ((char *)(_n_) + sizeof(*(_n_)))
#define ELFNOTE_DESC(_n_)   (ELFNOTE_NAME(_n_) + (((_n_)->namesz+3)&~3))
#define ELFNOTE_NEXT(_n_)   ((Elf_Note *)(ELFNOTE_DESC(_n_) + (((_n_)->descsz+3)&~3)))

would also do, and would even allow dropping an odd cast in the return statement
of xen_elfnote_string().

Jan

Re: Re: [patch] fix void* arithmetic

[Keir Fraser]

On 29/8/06 10:18 pm, "Hollis Blanchard" <hollisb@us.ibm.com> wrote:

>> Looks like PPC is the only arch using -Wpointer-arith, is there a reason
>> for that?
> 
> Is there are reason the other architectures *aren't* using it?
> 
> We have some extra warnings enabled because they've helped us in the
> past (such as -Wshadow). Given that we're just playing janitor for
> everyone else's code though, I think we're about to abandon that one.

We have code borrowed from Linux that doesn't like -Wpointer-arith very
much, so we took the pragmatic path here.

 -- Keir