All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen.5i5j@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	"rth@twiddle.net" <rth@twiddle.net>
Subject: Re: [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
Date: Fri, 10 Oct 2014 17:03:33 +0800	[thread overview]
Message-ID: <5437A0E5.1040200@gmail.com> (raw)
In-Reply-To: <54379E88.4050102@gmail.com>

On 10/10/14 16:53, Chen Gang wrote:
> On 10/10/14 15:37, Peter Maydell wrote:
>>>> The reason I'm reluctant to make changes to these files is
>>>> that they're pulled in from a different upstream project
>>>> (libvixl) so we should only fix critical problems in them,
>>>> or it makes new versions harder to update to.
>>>>
>>>
>>> Originally, I first try the Xilinx branch (Xilinx-master from Xilinx
>>> github), yesterday, and found this issue, then I try upstream main
>>> branch, found the same issue.
>>>
>>> For me, when add the related patch (which will use these variables in
>>> 'libvixl'), then declare and set them in the related headers, again.
>>> That will let other reviewers and readers easier understanding.
>>>
>>>  - removing them at present, is easy understanding.
>>>
>>>  - add them again when really need them, is also easy understanding.
>>
>> But it's all changes which we would have to carry locally
>> and then re-make every time we updated to a new libvixl.
>> I definitely don't want to do that unless it's absolutely
>> required.
>>
> 
> It is really a little complex, we almost can not touch this header file,
> sorry for my original misunderstanding.
> 
> And I guess, "disas/arm-64.cc" is our own file (only for qemu, not from
> libvixl upstream project). If really it is, we may do something in it to
> avoid this warning, e.g.
> 
>   "#pragma GCC diagnostic ignored -Wunused-variable" (almost like "include/ui/qemu-pixman.h" have done).
> 

Oh, may need "-Wunused-but-set-variable" instead of "-Wunused-variable",
and also need check CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE (I guess the
latest gcc should be OK).

If what I guess is OK, I shall try to test it under both the latest gcc
for x86_64 and the host gcc under Fedora 20 x86_64. If they are all OK,
I shall send patch v2 for it.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed


WARNING: multiple messages have this Message-ID (diff)
From: Chen Gang <gang.chen.5i5j@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	"rth@twiddle.net" <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror'
Date: Fri, 10 Oct 2014 17:03:33 +0800	[thread overview]
Message-ID: <5437A0E5.1040200@gmail.com> (raw)
In-Reply-To: <54379E88.4050102@gmail.com>

On 10/10/14 16:53, Chen Gang wrote:
> On 10/10/14 15:37, Peter Maydell wrote:
>>>> The reason I'm reluctant to make changes to these files is
>>>> that they're pulled in from a different upstream project
>>>> (libvixl) so we should only fix critical problems in them,
>>>> or it makes new versions harder to update to.
>>>>
>>>
>>> Originally, I first try the Xilinx branch (Xilinx-master from Xilinx
>>> github), yesterday, and found this issue, then I try upstream main
>>> branch, found the same issue.
>>>
>>> For me, when add the related patch (which will use these variables in
>>> 'libvixl'), then declare and set them in the related headers, again.
>>> That will let other reviewers and readers easier understanding.
>>>
>>>  - removing them at present, is easy understanding.
>>>
>>>  - add them again when really need them, is also easy understanding.
>>
>> But it's all changes which we would have to carry locally
>> and then re-make every time we updated to a new libvixl.
>> I definitely don't want to do that unless it's absolutely
>> required.
>>
> 
> It is really a little complex, we almost can not touch this header file,
> sorry for my original misunderstanding.
> 
> And I guess, "disas/arm-64.cc" is our own file (only for qemu, not from
> libvixl upstream project). If really it is, we may do something in it to
> avoid this warning, e.g.
> 
>   "#pragma GCC diagnostic ignored -Wunused-variable" (almost like "include/ui/qemu-pixman.h" have done).
> 

Oh, may need "-Wunused-but-set-variable" instead of "-Wunused-variable",
and also need check CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE (I guess the
latest gcc should be OK).

If what I guess is OK, I shall try to test it under both the latest gcc
for x86_64 and the host gcc under Fedora 20 x86_64. If they are all OK,
I shall send patch v2 for it.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

  reply	other threads:[~2014-10-10  8:58 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-09 14:00 [Qemu-trivial] [PATCH] disas/libvixl/a64/instructions-a64.h: Remove useless varialbe to avoid building break with '-Werror' Chen Gang
2014-10-09 14:00 ` [Qemu-devel] " Chen Gang
2014-10-09 14:34 ` [Qemu-trivial] " Peter Maydell
2014-10-09 14:34   ` [Qemu-devel] " Peter Maydell
2014-10-10  1:54   ` [Qemu-trivial] " Chen Gang
2014-10-10  1:54     ` [Qemu-devel] " Chen Gang
2014-10-10  7:37     ` [Qemu-trivial] " Peter Maydell
2014-10-10  7:37       ` [Qemu-devel] " Peter Maydell
2014-10-10  8:53       ` [Qemu-trivial] " Chen Gang
2014-10-10  8:53         ` [Qemu-devel] " Chen Gang
2014-10-10  9:03         ` Chen Gang [this message]
2014-10-10  9:03           ` Chen Gang
2014-10-09 14:54 ` [Qemu-trivial] " Eric Blake
2014-10-09 14:54   ` Eric Blake
2014-10-10  1:28   ` [Qemu-trivial] " Chen Gang
2014-10-10  1:28     ` Chen Gang
2014-10-21 15:50 ` [Qemu-trivial] " Peter Maydell
2014-10-21 15:50   ` [Qemu-devel] " Peter Maydell
2014-10-23  6:49   ` [Qemu-trivial] " Michael Tokarev
2014-10-23  6:49     ` [Qemu-devel] " Michael Tokarev
2014-10-23  7:14     ` Peter Maydell
2014-10-23  7:14       ` [Qemu-devel] " Peter Maydell
2014-10-23  8:09 ` Michael Tokarev
2014-10-23  8:09   ` [Qemu-devel] " Michael Tokarev
2014-10-23  9:02   ` Peter Maydell
2014-10-23  9:02     ` [Qemu-devel] " Peter Maydell
2014-10-23 10:09     ` Peter Maydell
2014-10-23 10:09       ` [Qemu-devel] " Peter Maydell
2014-10-23 10:27       ` Michael Tokarev
2014-10-23 10:27         ` [Qemu-devel] " Michael Tokarev
2014-10-23 11:05         ` Peter Maydell
2014-10-23 11:05           ` [Qemu-devel] " Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2014-10-21 23:27 Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5437A0E5.1040200@gmail.com \
    --to=gang.chen.5i5j@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.