[Qemu-devel] [PATCH] Use standard header for offsetof

[Qemu-devel] [PATCH] Use standard header for offsetof

[Stefan Weil]

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

This patch adds the standard header stddef.h to all files which call 
offsetof.
It also removes several local definitions of offsetof which are no 
longer needed.

If you prefer standard headers for standard macros instead of local 
definitions
(like I do), you can add this patch to Qemu trunk.

Thanks,
Stefan


[-- Attachment #2: offsetof.patch --]
[-- Type: text/x-diff, Size: 9456 bytes --]

Index: translate-all.c
===================================================================
--- translate-all.c	(revision 4638)
+++ translate-all.c	(working copy)
@@ -18,6 +18,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
Index: target-cris/translate.c
===================================================================
--- target-cris/translate.c	(revision 4638)
+++ target-cris/translate.c	(working copy)
@@ -25,6 +25,7 @@
  */
 
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
Index: softmmu_header.h
===================================================================
--- softmmu_header.h	(revision 4638)
+++ softmmu_header.h	(working copy)
@@ -17,6 +17,8 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
+
+#include <stddef.h>
 #if DATA_SIZE == 8
 #define SUFFIX q
 #define USUFFIX q
Index: target-sparc/translate.c
===================================================================
--- target-sparc/translate.c	(revision 4638)
+++ target-sparc/translate.c	(working copy)
@@ -20,6 +20,7 @@
  */
 
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
Index: target-m68k/helper.c
===================================================================
--- target-m68k/helper.c	(revision 4638)
+++ target-m68k/helper.c	(working copy)
@@ -19,6 +19,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#include <stddef.h>
 #include <stdio.h>
 #include <string.h>
 
Index: target-m68k/translate.c
===================================================================
--- target-m68k/translate.c	(revision 4638)
+++ target-m68k/translate.c	(working copy)
@@ -19,6 +19,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
Index: linux-user/signal.c
===================================================================
--- linux-user/signal.c	(revision 4638)
+++ linux-user/signal.c	(working copy)
@@ -17,6 +17,7 @@
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
@@ -548,10 +549,6 @@
     return ret;
 }
 
-#ifndef offsetof
-#define offsetof(type, field) ((size_t) &((type *)0)->field)
-#endif
-
 static inline int copy_siginfo_to_user(target_siginfo_t *tinfo,
                                        const target_siginfo_t *info)
 {
Index: target-mips/translate.c
===================================================================
--- target-mips/translate.c	(revision 4638)
+++ target-mips/translate.c	(working copy)
@@ -21,6 +21,7 @@
  */
 
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
Index: tcg/arm/tcg-target.c
===================================================================
--- tcg/arm/tcg-target.c	(revision 4638)
+++ tcg/arm/tcg-target.c	(working copy)
@@ -21,6 +21,9 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
+#include <stddef.h>
+
 const char *tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%r0",
     "%r1",
Index: tcg/ppc/tcg-target.c
===================================================================
--- tcg/ppc/tcg-target.c	(revision 4638)
+++ tcg/ppc/tcg-target.c	(working copy)
@@ -22,6 +22,8 @@
  * THE SOFTWARE.
  */
 
+#include <stddef.h>
+
 static uint8_t *tb_ret_addr;
 
 #define FAST_PATH
Index: tcg/hppa/tcg-target.c
===================================================================
--- tcg/hppa/tcg-target.c	(revision 4638)
+++ tcg/hppa/tcg-target.c	(working copy)
@@ -22,6 +22,8 @@
  * THE SOFTWARE.
  */
 
+#include <stddef.h>
+
 static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%r0",
     "%r1",
Index: tcg/i386/tcg-target.c
===================================================================
--- tcg/i386/tcg-target.c	(revision 4638)
+++ tcg/i386/tcg-target.c	(working copy)
@@ -21,6 +21,9 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
+#include <stddef.h>
+
 const char *tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%eax",
     "%ecx",
Index: tcg/x86_64/tcg-target.c
===================================================================
--- tcg/x86_64/tcg-target.c	(revision 4638)
+++ tcg/x86_64/tcg-target.c	(working copy)
@@ -21,6 +21,9 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
+#include <stddef.h>
+
 const char *tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%rax",
     "%rcx",
Index: tcg/sparc/tcg-target.c
===================================================================
--- tcg/sparc/tcg-target.c	(revision 4638)
+++ tcg/sparc/tcg-target.c	(working copy)
@@ -22,6 +22,8 @@
  * THE SOFTWARE.
  */
 
+#include <stddef.h>
+
 static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%g0",
     "%g1",
Index: cpu-exec.c
===================================================================
--- cpu-exec.c	(revision 4638)
+++ cpu-exec.c	(working copy)
@@ -17,6 +17,7 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
+#include <stddef.h>
 #include "config.h"
 #define CPU_NO_GLOBAL_REGS
 #include "exec.h"
Index: block-qcow2.c
===================================================================
--- block-qcow2.c	(revision 4638)
+++ block-qcow2.c	(working copy)
@@ -21,6 +21,8 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
+#include <stddef.h>
 #include "qemu-common.h"
 #include "block_int.h"
 #include <zlib.h>
@@ -59,10 +61,6 @@
 
 #define REFCOUNT_SHIFT 1 /* refcount size is 2 bytes */
 
-#ifndef offsetof
-#define offsetof(type, field) ((size_t) &((type *)0)->field)
-#endif
-
 typedef struct QCowHeader {
     uint32_t magic;
     uint32_t version;
Index: exec.c
===================================================================
--- exec.c	(revision 4638)
+++ exec.c	(working copy)
@@ -28,6 +28,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <stddef.h>
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
Index: monitor.c
===================================================================
--- monitor.c	(revision 4638)
+++ monitor.c	(working copy)
@@ -21,6 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include <stddef.h>
 #include "hw/hw.h"
 #include "hw/usb.h"
 #include "hw/pcmcia.h"
@@ -43,10 +44,6 @@
 //#define DEBUG
 //#define DEBUG_COMPLETION
 
-#ifndef offsetof
-#define offsetof(type, field) ((size_t) &((type *)0)->field)
-#endif
-
 /*
  * Supported types:
  *
Index: exec-all.h
===================================================================
--- exec-all.h	(revision 4638)
+++ exec-all.h	(working copy)
@@ -275,10 +275,6 @@
 
 TranslationBlock *tb_find_pc(unsigned long pc_ptr);
 
-#ifndef offsetof
-#define offsetof(type, field) ((size_t) &((type *)0)->field)
-#endif
-
 #if defined(_WIN32)
 #define ASM_DATA_SECTION ".section \".data\"\n"
 #define ASM_PREVIOUS_SECTION ".section .text\n"
Index: target-i386/helper.c
===================================================================
--- target-i386/helper.c	(revision 4638)
+++ target-i386/helper.c	(working copy)
@@ -18,6 +18,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
Index: target-i386/op_helper.c
===================================================================
--- target-i386/op_helper.c	(revision 4638)
+++ target-i386/op_helper.c	(working copy)
@@ -18,6 +18,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #define CPU_NO_GLOBAL_REGS
+#include <stddef.h>
 #include "exec.h"
 #include "host-utils.h"
 
Index: target-i386/translate.c
===================================================================
--- target-i386/translate.c	(revision 4638)
+++ target-i386/translate.c	(working copy)
@@ -18,6 +18,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
Index: target-arm/helper.c
===================================================================
--- target-arm/helper.c	(revision 4638)
+++ target-arm/helper.c	(working copy)
@@ -1,4 +1,5 @@
 #include <stdio.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
 
Index: target-arm/translate.c
===================================================================
--- target-arm/translate.c	(revision 4638)
+++ target-arm/translate.c	(working copy)
@@ -20,6 +20,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Johannes Schindelin]

Hi,

On Sun, 1 Jun 2008, Stefan Weil wrote:

> This patch adds the standard header stddef.h to all files which call 
> offsetof. It also removes several local definitions of offsetof which 
> are no longer needed.

It would have been nice to provide some defense for that patch.  For 
example just mentioning that offsetof is in C89 would have spared me a 
bit of research.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Anthony Liguori]

Stefan Weil wrote:
> This patch adds the standard header stddef.h to all files which call 
> offsetof.
> It also removes several local definitions of offsetof which are no 
> longer needed.
>
> If you prefer standard headers for standard macros instead of local 
> definitions
> (like I do), you can add this patch to Qemu trunk.

I don't understand why you're adding #include <stdef.h> to files that do 
not define offsetof.  What's the rationale for that?

Regards,

Anthony Liguori

> Thanks,
> Stefan
>

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Stefan Weil]

Anthony Liguori schrieb:
>
> I don't understand why you're adding #include <stdef.h> to files that 
> do not define offsetof.  What's the rationale for that?
>
> Regards,
>
> Anthony Liguori
>
There were several possible ways to replace the defines for offsetof.
After removing the defines, I could

1) include stddef.h at the places where offsetof was defined formerly

or

2) include stddef.h at the places where offsetof was used

or

3) include stddef.h for all files where the compiler complains because 
of missing declaration for offsetof

I prefered solution 2 because it minimizes dependencies and usage of 
include files.
Solution 3 would have needed a complete compile test (all possible 
targets for all
possible hosts and all possible configuration options).

Normally, I also add a comment behind any include statement which shows 
the reason for it:

#include <stddef.h>   /* offsetof */

So everybody can see why the include is there, and the maintainer can 
remove it
when the former reason is no longer valid.
But this is not Qemu-like style, so I did not use it here.

Regards,
Stefan

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Anthony Liguori]

Stefan Weil wrote:
> Anthony Liguori schrieb:
>>
>> I don't understand why you're adding #include <stdef.h> to files that 
>> do not define offsetof.  What's the rationale for that?
>>
>> Regards,
>>
>> Anthony Liguori
>>
> There were several possible ways to replace the defines for offsetof.
> After removing the defines, I could
>
> 1) include stddef.h at the places where offsetof was defined formerly

This seems like the most logically thing to do to me.  Otherwise, you're 
unnecessarily increasing the number of #include's in C files.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Stefan Weil]

Anthony Liguori schrieb:
> Stefan Weil wrote:
>> Anthony Liguori schrieb:
>>>
>>> I don't understand why you're adding #include <stdef.h> to files 
>>> that do not define offsetof.  What's the rationale for that?
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>> There were several possible ways to replace the defines for offsetof.
>> After removing the defines, I could
>>
>> 1) include stddef.h at the places where offsetof was defined formerly
>
> This seems like the most logically thing to do to me.  Otherwise, 
> you're unnecessarily increasing the number of #include's in C files.
>

exec-all.h is one of these places. Adding #include <stddef.h> there 
reduces the number of #include statements
in C sources, but now all sources which need exec-all.h also include 
stddef.h during compilation.
So this increases the number of included headers during a compilation.

If you prefer source files without many includes, you can put all system 
includes in some central project headers.
Many projects do this, and Qemu's qemu-common.h is an example for this 
approach. You could add the #include
for stddef.h there.

I prefer minimized dependencies and short compile times, so I include 
system headers only at the places which need them.

The choice between both alternatives also depends on your compiler: 
nowadays most C compilers are clever enough
not to parse a system header file more than once while compiling a C 
source which has several references to it.
This is in favour of my prefered choice. But other compilers use 
precompiled headers, and for MS C with precompiled
headers, few large header files are better. So there is no clear winner 
for all situations.

Qemu has no clear strategy for header file inclusion: qemu-common.h is 
used by many source files and includes stdio.h,
but some files (for example osdep.h) include both qemu-common.h and stdio.h.

Regards
Stefan

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Andreas Färber]

Am 06.06.2008 um 21:44 schrieb Stefan Weil:

> Qemu has no clear strategy for header file inclusion: qemu-common.h  
> is used by many source files and includes stdio.h,
> but some files (for example osdep.h) include both qemu-common.h and  
> stdio.h.

Iirc, qemu-common.h is still relatively new. If you noticed duplicate  
inclusions, those may be for "historic" reasons, so you could provide  
a patch eliminating them.

Andreas

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Ian Jackson]

Andreas Färber writes ("Re: [Qemu-devel] [PATCH] Use standard header for offsetof"):
> Am 06.06.2008 um 21:44 schrieb Stefan Weil:
> > Qemu has no clear strategy for header file inclusion: qemu-common.h  
> > is used by many source files and includes stdio.h,
> > but some files (for example osdep.h) include both qemu-common.h and  
> > stdio.h.
> 
> Iirc, qemu-common.h is still relatively new. If you noticed duplicate  
> inclusions, those may be for "historic" reasons, so you could provide  
> a patch eliminating them.

If qemu-common.h is the new approach, then perhaps Stefan should
resubmit a version of his patch to add <stddef.h> to qemu-common.h,
rather than to all of the .c files which use offsetof ?

Ian.

Re: [Qemu-devel] [PATCH] Use standard header for offsetof

[Anthony Liguori]

Stefan Weil wrote:
> Anthony Liguori schrieb:
>> Stefan Weil wrote:
>>> Anthony Liguori schrieb:
>>>>
>>>> I don't understand why you're adding #include <stdef.h> to files 
>>>> that do not define offsetof.  What's the rationale for that?
>>>>
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>>>
>>> There were several possible ways to replace the defines for offsetof.
>>> After removing the defines, I could
>>>
>>> 1) include stddef.h at the places where offsetof was defined formerly
>>
>> This seems like the most logically thing to do to me.  Otherwise, 
>> you're unnecessarily increasing the number of #include's in C files.
>>
>
> exec-all.h is one of these places. Adding #include <stddef.h> there 
> reduces the number of #include statements
> in C sources, but now all sources which need exec-all.h also include 
> stddef.h during compilation.
> So this increases the number of included headers during a compilation.

If you want to take a stab at improving the #include's in QEMU, then you 
should do that as a separate patch.  I'll warn you though that I doubt 
there's a lot of people interested in that and I think there's always 
reservations about changing headers because it often causes subtle 
breakages.

Don't complicate what should be a simple patch to use the standard 
offset of though with other clean-ups.

Regards,

Anthony Liguori