Re: [Qemu-trivial] [Qemu-devel] [PATCH] disas/arm: Remove redefinition of ATTRIBUTE_UNUSED

[Tobias Klauser]

On 2014-09-24 at 11:09:42 +0200, Peter Maydell <address@hidden> wrote:
> On 24 September 2014 00:58, Michael Tokarev <address@hidden> wrote:
> > 18.09.2014 21:25, Tobias Klauser wrote:
> >> ATTRIBUTE_UNUSED is already defined in disas/bfd.h, which is included.
> >> Thus, there is no need to redefine it.
> >
> > Is there any harm in keeping it here?
> >
> > While it really is a redifinition, I'm not sure what's the right thing
> > here.  This whole code is not from qemu, it is an external source imported,
> > and that source is being maintained (but under different license as has
> > already been discussed, so keeping changes at minimum might not be that
> > good idea anymore).  On the other hand this symbol is so common it should
> > be defined in a common header.  Yet on another hand, for these external
> > sources wich has public API, it might not be a good idea to define it in
> > a header to start with, because it might clash with project-local define,
> > so it might be better to define it in either private header or in individual
> > source.
> 
> So my take on the disas/ sources is:
>  * yes, they're from an external source
>  * but as you say, we're never going to take another drop from that
>    external source so we should feel free to make local bugfixes
>    and changes as we need to
>  * on the other hand, there's no point in making gratuitous changes
>    to them (they're never going to match the QEMU coding style,
>    for instance)
> 
> So what's the rationale for this particular change? The duplication
> is harmless, so why worry about it...

There's no particular reason, I just found it odd to see a redefinition
of the macro there and I didn't know about the external source of the
code - please just ignore the patch then.

Thanks
Tobias