qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH] arm/translate-a64: mark path as unreachable to el


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-arm] [PATCH] arm/translate-a64: mark path as unreachable to eliminate warning
Date: Tue, 5 Dec 2017 01:34:46 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

Hi Peter,

On 11/13/2017 08:26 AM, Peter Maydell wrote:
> On 8 November 2017 at 12:37, Philippe Mathieu-Daudé <address@hidden> wrote:
>> On 11/07/2017 05:46 PM, Emilio G. Cota wrote:
>>> Fixes the following warning when compiling with gcc 5.4.0 with -O1
>>> optimizations and --enable-debug:
>>>
>>> target/arm/translate-a64.c: In function ‘aarch64_tr_translate_insn’:
>>> target/arm/translate-a64.c:2361:8: error: ‘post_index’ may be used 
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>      if (!post_index) {
>>>         ^
>>> target/arm/translate-a64.c:2307:10: note: ‘post_index’ was declared here
>>>      bool post_index;
>>>           ^
>>> target/arm/translate-a64.c:2386:8: error: ‘writeback’ may be used 
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>      if (writeback) {
>>>         ^
>>> target/arm/translate-a64.c:2308:10: note: ‘writeback’ was declared here
>>>      bool writeback;
>>>           ^
>>>
>>> Note that idx comes from selecting 2 bits, and therefore its value
>>> can be at most 3.
>>>
>>> Signed-off-by: Emilio G. Cota <address@hidden>
> 
> Applied to target-arm.next, thanks. (It's a bit sad that
> gcc can't figure out that the result of x & 3 is constrained
> to [0,3], but there you go.)
> 
>> Acked-by: Philippe Mathieu-Daudé <address@hidden>
> 
> By the way, Philippe, what are you intending to convey with
> an acked-by tag? Usually "acked-by" means "I'm the maintainer
> for this area and I haven't actually reviewed this but I don't
> object to it"...

I still feel new into the FOSS community and am happy to learn and
understand from such comments :)
Reading thru the list I wondered what means this tag, asked to some
maintainers on IRC and read the Linux kernel "Submitting patches guide"
[1] bullet 12) "When to use Acked-by":

'''
Acked-by: is often used by the maintainer of the affected code when that
maintainer neither contributed to nor forwarded the patch.

Acked-by: is not as formal as Signed-off-by:. It is a record that the
acker has at least reviewed the patch and has indicated acceptance.
Hence patch mergers will sometimes manually convert an acker’s “yep,
looks good to me” into an Acked-by: (but note that it is usually better
to ask for an explicit ack).

Acked-by: does not necessarily indicate acknowledgement of the entire
patch. For example, if a patch affects multiple subsystems and has an
Acked-by: from one subsystem maintainer then this usually indicates
acknowledgement of just the part which affects that maintainer’s code. [...]
'''

It is not obvious that the 2nd case is restricted to maintainer, but now
than you explain it I understand.

I intended to express "I don't think this is the best way to solve this
problem, however this is probably the best fix when freezed for a
Release, and since I did have reviewed this in details (and tested it)
but I don't object to it, instead of signing with my Reviewed-by tag I
lower it to an Acked-by".

Next time I'll keep it simple and use a R-b :)

Thanks to take time to correct me!

Regards,

Phil.

[1]
https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html#when-to-use-acked-by-and-cc



reply via email to

[Prev in Thread] Current Thread [Next in Thread]