diff mbox

libvixl: Correct ordering of includes and fix NetBSD build

Message ID 20170513015449.24061-1-n54@gmx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kamil Rytarowski May 13, 2017, 1:54 a.m. UTC
The __STDC_CONSTANT_MACROS symbol must be defined before including
directly or indirectly <stdint.h> in order to get support for macros
for integer constants like INT8_C().

The vixl/globals.h headers defines __STDC_CONSTANT_MACROS and must be
included before other system headers.

This change fixes build failures on NetBSD.

Signed-off-by: Kamil Rytarowski <n54@gmx.com>
---
 disas/libvixl/vixl/a64/disasm-a64.cc | 2 +-
 disas/libvixl/vixl/utils.h           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Philippe Mathieu-Daudé May 13, 2017, 10:04 p.m. UTC | #1
Hi Kamil,

I think it is safer to add it in disas/libvixl/Makefile.objs where 
QEMU_CFLAGS are tuned for libvixl.
This way you don't need to modify upstream libvixl.

Regards,

Phil.

On 05/12/2017 10:54 PM, Kamil Rytarowski wrote:
> The __STDC_CONSTANT_MACROS symbol must be defined before including
> directly or indirectly <stdint.h> in order to get support for macros
> for integer constants like INT8_C().
>
> The vixl/globals.h headers defines __STDC_CONSTANT_MACROS and must be
> included before other system headers.
>
> This change fixes build failures on NetBSD.
>
> Signed-off-by: Kamil Rytarowski <n54@gmx.com>
> ---
>  disas/libvixl/vixl/a64/disasm-a64.cc | 2 +-
>  disas/libvixl/vixl/utils.h           | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/disas/libvixl/vixl/a64/disasm-a64.cc b/disas/libvixl/vixl/a64/disasm-a64.cc
> index 7a58a5c087..fc87306893 100644
> --- a/disas/libvixl/vixl/a64/disasm-a64.cc
> +++ b/disas/libvixl/vixl/a64/disasm-a64.cc
> @@ -24,8 +24,8 @@
>  // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>  // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>
> -#include <cstdlib>
>  #include "vixl/a64/disasm-a64.h"
> +#include <cstdlib>
>
>  namespace vixl {
>
> diff --git a/disas/libvixl/vixl/utils.h b/disas/libvixl/vixl/utils.h
> index 5ab134e240..17034addbc 100644
> --- a/disas/libvixl/vixl/utils.h
> +++ b/disas/libvixl/vixl/utils.h
> @@ -27,10 +27,10 @@
>  #ifndef VIXL_UTILS_H
>  #define VIXL_UTILS_H
>
> -#include <string.h>
> -#include <cmath>
>  #include "vixl/globals.h"
>  #include "vixl/compiler-intrinsics.h"
> +#include <string.h>
> +#include <cmath>
>
>  namespace vixl {
>
>
Kamil Rytarowski May 13, 2017, 10:15 p.m. UTC | #2
Hello,

I will give it a try and submit as a new patch.

On 14.05.2017 00:04, Philippe Mathieu-Daudé wrote:
> Hi Kamil,
> 
> I think it is safer to add it in disas/libvixl/Makefile.objs where
> QEMU_CFLAGS are tuned for libvixl.
> This way you don't need to modify upstream libvixl.
> 
> Regards,
> 
> Phil.
> 
> On 05/12/2017 10:54 PM, Kamil Rytarowski wrote:
>> The __STDC_CONSTANT_MACROS symbol must be defined before including
>> directly or indirectly <stdint.h> in order to get support for macros
>> for integer constants like INT8_C().
>>
>> The vixl/globals.h headers defines __STDC_CONSTANT_MACROS and must be
>> included before other system headers.
>>
>> This change fixes build failures on NetBSD.
>>
>> Signed-off-by: Kamil Rytarowski <n54@gmx.com>
>> ---
>>  disas/libvixl/vixl/a64/disasm-a64.cc | 2 +-
>>  disas/libvixl/vixl/utils.h           | 4 ++--
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/disas/libvixl/vixl/a64/disasm-a64.cc
>> b/disas/libvixl/vixl/a64/disasm-a64.cc
>> index 7a58a5c087..fc87306893 100644
>> --- a/disas/libvixl/vixl/a64/disasm-a64.cc
>> +++ b/disas/libvixl/vixl/a64/disasm-a64.cc
>> @@ -24,8 +24,8 @@
>>  // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
>> OF THE USE
>>  // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>
>> -#include <cstdlib>
>>  #include "vixl/a64/disasm-a64.h"
>> +#include <cstdlib>
>>
>>  namespace vixl {
>>
>> diff --git a/disas/libvixl/vixl/utils.h b/disas/libvixl/vixl/utils.h
>> index 5ab134e240..17034addbc 100644
>> --- a/disas/libvixl/vixl/utils.h
>> +++ b/disas/libvixl/vixl/utils.h
>> @@ -27,10 +27,10 @@
>>  #ifndef VIXL_UTILS_H
>>  #define VIXL_UTILS_H
>>
>> -#include <string.h>
>> -#include <cmath>
>>  #include "vixl/globals.h"
>>  #include "vixl/compiler-intrinsics.h"
>> +#include <string.h>
>> +#include <cmath>
>>
>>  namespace vixl {
>>
>>
>
Eric Blake May 15, 2017, 1:57 p.m. UTC | #3
On 05/13/2017 05:04 PM, Philippe Mathieu-Daudé wrote:
> Hi Kamil,
> 
> I think it is safer to add it in disas/libvixl/Makefile.objs where
> QEMU_CFLAGS are tuned for libvixl.
> This way you don't need to modify upstream libvixl.
> 

Ah, right. disas/libvixl is one of the directories exempt from our
normal rules of including osdep.h first (otherwise I would have said
that including stdint.h first should be the job of osdep.h).

But indeed, that's because we are trying to leave libvixl as untouched
as possible, so if there IS a solution that can be done through
makefiles rather than direct file editing, it is worth considering.
Kamil Rytarowski May 15, 2017, 1:57 p.m. UTC | #4
On 15.05.2017 15:57, Eric Blake wrote:
> On 05/13/2017 05:04 PM, Philippe Mathieu-Daudé wrote:
>> Hi Kamil,
>>
>> I think it is safer to add it in disas/libvixl/Makefile.objs where
>> QEMU_CFLAGS are tuned for libvixl.
>> This way you don't need to modify upstream libvixl.
>>
> 
> Ah, right. disas/libvixl is one of the directories exempt from our
> normal rules of including osdep.h first (otherwise I would have said
> that including stdint.h first should be the job of osdep.h).
> 
> But indeed, that's because we are trying to leave libvixl as untouched
> as possible, so if there IS a solution that can be done through
> makefiles rather than direct file editing, it is worth considering.
> 

http://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg03321.html
diff mbox

Patch

diff --git a/disas/libvixl/vixl/a64/disasm-a64.cc b/disas/libvixl/vixl/a64/disasm-a64.cc
index 7a58a5c087..fc87306893 100644
--- a/disas/libvixl/vixl/a64/disasm-a64.cc
+++ b/disas/libvixl/vixl/a64/disasm-a64.cc
@@ -24,8 +24,8 @@ 
 // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-#include <cstdlib>
 #include "vixl/a64/disasm-a64.h"
+#include <cstdlib>
 
 namespace vixl {
 
diff --git a/disas/libvixl/vixl/utils.h b/disas/libvixl/vixl/utils.h
index 5ab134e240..17034addbc 100644
--- a/disas/libvixl/vixl/utils.h
+++ b/disas/libvixl/vixl/utils.h
@@ -27,10 +27,10 @@ 
 #ifndef VIXL_UTILS_H
 #define VIXL_UTILS_H
 
-#include <string.h>
-#include <cmath>
 #include "vixl/globals.h"
 #include "vixl/compiler-intrinsics.h"
+#include <string.h>
+#include <cmath>
 
 namespace vixl {