[Qemu-devel] [PATCH] osdep: Remove local definition of macro offsetof

[Qemu-devel] [PATCH] osdep: Remove local definition of macro offsetof

[Stefan Weil]

The macro offsetof is defined in stddef.h. It is conforming to
the standards C89, C99 and POSIX.1-2001 (see man page), so it
is a sufficiently old standard.

Therefore chances are very high that QEMU never needs a local
definition of this macro.

osdep.h already includes stddef.h, so this patch simply removes
the unneeded code from the files configure and osdep.h.

If we ever need the local definition again, it should be added
to compiler.h (the macro is usually provided with the compiler,
it is not OS specific).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 configure |   14 --------------
 osdep.h   |    3 ---
 2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/configure b/configure
index b607795..a260fab 100755
--- a/configure
+++ b/configure
@@ -2525,17 +2525,6 @@ if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
 fi
 
 ##########################################
-# check if the compiler defines offsetof
-
-need_offsetof=yes
-cat > $TMPC << EOF
-#include <stddef.h>
-int main(void) { struct s { int f; }; return offsetof(struct s, f); }
-EOF
-if compile_prog "" "" ; then
-    need_offsetof=no
-fi
-
 # spice probe
 if test "$spice" != "no" ; then
   cat > $TMPC << EOF
@@ -3200,9 +3189,6 @@ fi
 if test "$tcg_interpreter" = "yes" ; then
   echo "CONFIG_TCG_INTERPRETER=y" >> $config_host_mak
 fi
-if test "$need_offsetof" = "yes" ; then
-  echo "CONFIG_NEED_OFFSETOF=y" >> $config_host_mak
-fi
 if test "$fdatasync" = "yes" ; then
   echo "CONFIG_FDATASYNC=y" >> $config_host_mak
 fi
diff --git a/osdep.h b/osdep.h
index 432b91e..0350383 100644
--- a/osdep.h
+++ b/osdep.h
@@ -26,9 +26,6 @@
 #define unlikely(x)   __builtin_expect(!!(x), 0)
 #endif
 
-#ifdef CONFIG_NEED_OFFSETOF
-#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *) 0)->MEMBER)
-#endif
 #ifndef container_of
 #define container_of(ptr, type, member) ({                      \
         const typeof(((type *) 0)->member) *__mptr = (ptr);     \
-- 
1.7.9

Re: [Qemu-devel] [PATCH] osdep: Remove local definition of macro offsetof

[Andreas Färber]

Am 05.03.2012 07:22, schrieb Stefan Weil:
> The macro offsetof is defined in stddef.h. It is conforming to
> the standards C89, C99 and POSIX.1-2001 (see man page), so it
> is a sufficiently old standard.
> 
> Therefore chances are very high that QEMU never needs a local
> definition of this macro.
> 
> osdep.h already includes stddef.h, so this patch simply removes
> the unneeded code from the files configure and osdep.h.
> 
> If we ever need the local definition again, it should be added
> to compiler.h (the macro is usually provided with the compiler,
> it is not OS specific).
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>

Did you check when this was introduced and whether the commit message
gave any explanation why?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] osdep: Remove local definition of macro offsetof

[Peter Maydell]

On 5 March 2012 13:23, Andreas Färber <afaerber@suse.de> wrote:
> Am 05.03.2012 07:22, schrieb Stefan Weil:
>> The macro offsetof is defined in stddef.h. It is conforming to
>> the standards C89, C99 and POSIX.1-2001 (see man page), so it
>> is a sufficiently old standard.
>>
>> Therefore chances are very high that QEMU never needs a local
>> definition of this macro.

> Did you check when this was introduced and whether the commit message
> gave any explanation why?

It's been copied and moved around and rerationalised down to one
definition since then, but the first offsetof() definition was
added by Fabrice in fd6ce8f66 in May 2003, with no particular
comment about it. That is about as close as qemu gets to "it has
always been this way" :-)

Nearly a decade on, I think dropping it is probably reasonably
safe.

-- PMM

Re: [Qemu-devel] [Qemu-trivial] [PATCH] osdep: Remove local definition of macro offsetof

[Stefan Hajnoczi]

On Mon, Mar 05, 2012 at 07:22:31AM +0100, Stefan Weil wrote:
> The macro offsetof is defined in stddef.h. It is conforming to
> the standards C89, C99 and POSIX.1-2001 (see man page), so it
> is a sufficiently old standard.
> 
> Therefore chances are very high that QEMU never needs a local
> definition of this macro.
> 
> osdep.h already includes stddef.h, so this patch simply removes
> the unneeded code from the files configure and osdep.h.
> 
> If we ever need the local definition again, it should be added
> to compiler.h (the macro is usually provided with the compiler,
> it is not OS specific).
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  configure |   14 --------------
>  osdep.h   |    3 ---
>  2 files changed, 0 insertions(+), 17 deletions(-)

Andreas and Peter, thanks for looking into whether it's safe to remove
offsetof().  I feel the risk is very low.

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

Re: [Qemu-devel] [PATCH] osdep: Remove local definition of macro offsetof

[Stefan Weil]

Am 05.03.2012 14:49, schrieb Peter Maydell:
> On 5 March 2012 13:23, Andreas Färber <afaerber@suse.de> wrote:
>> Am 05.03.2012 07:22, schrieb Stefan Weil:
>>> The macro offsetof is defined in stddef.h. It is conforming to
>>> the standards C89, C99 and POSIX.1-2001 (see man page), so it
>>> is a sufficiently old standard.
>>>
>>> Therefore chances are very high that QEMU never needs a local
>>> definition of this macro.
>
>> Did you check when this was introduced and whether the commit message
>> gave any explanation why?
>
> It's been copied and moved around and rerationalised down to one
> definition since then, but the first offsetof() definition was
> added by Fabrice in fd6ce8f66 in May 2003, with no particular
> comment about it. That is about as close as qemu gets to "it has
> always been this way" :-)
>
> Nearly a decade on, I think dropping it is probably reasonably
> safe.
>
> -- PMM

Thanks for your mail.

If anybody likes historic discussion threads, here is one from 2008:

http://lists.nongnu.org/archive/html/qemu-devel/2008-06/msg00009.html

I am working without the local definition in my QEMU tree since 4 years now.

Cheers,
Stefan W.