Re: [Qemu-devel] [PATCH 17/17] include: Move fprintf_function to disas/

[Markus Armbruster <armbru@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> The previous commits have eliminated fprintf_function outside
>> disassemblers, simplifying code and cleaning up the ugly type-punning
>> fprintf_function seems to attract.  Move fprintf_function to
>> include/disas/dis-asm.h to reduce the temptation to abuse it.
>> 
>> I considered renaming it to fprintf_ftype (reverting that part of
>> commit 6e2d864edf5, v0.14.0) to get us closer to binutils, but I
>> figure the fork is too distant to make this worthwhile.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> I'm OKish with this - although it's the only place we currently
> use it, it's a fairly common concept, so I'm not sure it's worth
> banishing.

I found 15 uses, ranging from questionable to outright bad.  I
eliminated 14, and cleaned up one.  I guess that has made me skeptical
on the chances of this type getting used sanely.

Banishing the type to where it came from reduces the temptation.

Perhaps that's not actually necessary now we've gotten rid of the bad
examples.

Should a sane use come up, we can simply revert this patch.

> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks!