Use standard header for offsetof

PATCH In review Stefan Weil 5 participants 9 messages Jun 1, 2008 – Jun 9, 2008

Share & tools

Stefan Weil

author

2008-06-01 17:37 UTC #0 lore raw

PATCH 1/1 Use standard header for offsetof

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

translate-all.c +1 -0

Index: translate-all.c
===================================================================
@@ -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>

translate.c +1 -0

Index: target-cris/translate.c
===================================================================
@@ -25,6 +25,7 @@
  */
 
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>

softmmu_header.h +2 -0

Index: softmmu_header.h
===================================================================
@@ -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

translate.c +1 -0

Index: target-sparc/translate.c
===================================================================
@@ -20,6 +20,7 @@
  */
 
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>

helper.c +1 -0

Index: target-m68k/helper.c
===================================================================
@@ -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>

translate.c +1 -0

Index: target-m68k/translate.c
===================================================================
@@ -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>

signal.c +1 -4

Index: linux-user/signal.c
===================================================================
@@ -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)
 {

translate.c +1 -0

Index: target-mips/translate.c
===================================================================
@@ -21,6 +21,7 @@
  */
 
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>

arm/tcg-target.c +3 -0

Index: tcg/arm/tcg-target.c
===================================================================
@@ -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",

ppc/tcg-target.c +2 -0

Index: tcg/ppc/tcg-target.c
===================================================================
@@ -22,6 +22,8 @@
  * THE SOFTWARE.
  */
 
+#include <stddef.h>
+
 static uint8_t *tb_ret_addr;
 
 #define FAST_PATH

hppa/tcg-target.c +2 -0

Index: tcg/hppa/tcg-target.c
===================================================================
@@ -22,6 +22,8 @@
  * THE SOFTWARE.
  */
 
+#include <stddef.h>
+
 static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%r0",
     "%r1",

i386/tcg-target.c +3 -0

Index: tcg/i386/tcg-target.c
===================================================================
@@ -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",

x86_64/tcg-target.c +3 -0

Index: tcg/x86_64/tcg-target.c
===================================================================
@@ -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",

sparc/tcg-target.c +2 -0

Index: tcg/sparc/tcg-target.c
===================================================================
@@ -22,6 +22,8 @@
  * THE SOFTWARE.
  */
 
+#include <stddef.h>
+
 static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%g0",
     "%g1",

cpu-exec.c +1 -0

Index: cpu-exec.c
===================================================================
@@ -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"

block-qcow2.c +2 -4

Index: block-qcow2.c
===================================================================
@@ -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;

exec.c +1 -0

Index: exec.c
===================================================================
@@ -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>

monitor.c +1 -4

Index: monitor.c
===================================================================
@@ -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:
  *

exec-all.h +0 -4

Index: exec-all.h
===================================================================
@@ -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"

helper.c +1 -0

Index: target-i386/helper.c
===================================================================
@@ -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>

op_helper.c +1 -0

Index: target-i386/op_helper.c
===================================================================
@@ -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"

translate.c +1 -0

Index: target-i386/translate.c
===================================================================
@@ -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>

helper.c +1 -0

Index: target-arm/helper.c
===================================================================
@@ -1,4 +1,5 @@
 #include <stdio.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <string.h>

translate.c +1 -0

Index: target-arm/translate.c
===================================================================
@@ -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>

Johannes Schindelin

2008-06-02 06:23 UTC #1 lore raw

Hi,

Quoted 3 lines · Stefan Weil

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

Anthony Liguori

2008-06-03 19:09 UTC #2 lore raw

In reply to Stefan Weil

Quoted 5 lines · Stefan Weil

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

Quoted 3 lines

Thanks,
Stefan

Stefan Weil

author

2008-06-03 21:03 UTC #3 lore raw

Quoted 7 lines · Anthony Liguori

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

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

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

Anthony Liguori

2008-06-03 21:25 UTC #4 lore raw

Quoted 4 lines · Stefan Weil

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

Quoted 8 lines

Quoted 4 lines
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

Stefan Weil

author

2008-06-06 19:45 UTC #5 lore raw

Quoted 7 lines · Anthony Liguori

Quoted 6 lines · Stefan Weil
Quoted 5 lines · Anthony Liguori
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,

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
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

Andreas Färber

2008-06-06 19:58 UTC #6 lore raw

Am 06.06.2008 um 21:44 schrieb Stefan Weil:

Quoted 2 lines

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

Ian Jackson

2008-06-09 10:30 UTC #7 lore raw

Andreas Färber writes ("Re: [Qemu-devel] [PATCH] Use standard header for offsetof"):

Quoted 9 lines

Am 06.06.2008 um 21:44 schrieb Stefan Weil:
Quoted 4 lines
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.

Anthony Liguori

2008-06-06 20:32 UTC #8 lore raw

In reply to Stefan Weil

Quoted 13 lines · Stefan Weil

Quoted 12 lines · Anthony Liguori
Quoted 11 lines · Stefan Weil
Quoted 7 lines · Anthony Liguori
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

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
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