[Qemu-devel] [PATCH] libvixl: Correct ordering of includes and fix NetBSD build

Kamil Rytarowski posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170513015449.24061-1-n54@gmx.com
Test checkpatch passed
Test docker passed
Test s390x passed
disas/libvixl/vixl/a64/disasm-a64.cc | 2 +-
disas/libvixl/vixl/utils.h           | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] libvixl: Correct ordering of includes and fix NetBSD build
Posted by Kamil Rytarowski 6 years, 11 months ago
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 {
 
-- 
2.12.2


Re: [Qemu-devel] [PATCH] libvixl: Correct ordering of includes and fix NetBSD build
Posted by Philippe Mathieu-Daudé 6 years, 11 months ago
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 {
>
>

Re: [Qemu-devel] [PATCH] libvixl: Correct ordering of includes and fix NetBSD build
Posted by Kamil Rytarowski 6 years, 11 months ago
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 {
>>
>>
> 


Re: [Qemu-devel] [PATCH] libvixl: Correct ordering of includes and fix NetBSD build
Posted by Eric Blake 6 years, 11 months ago
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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] libvixl: Correct ordering of includes and fix NetBSD build
Posted by Kamil Rytarowski 6 years, 11 months ago
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