[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] fprintf-fn.h uses GCC_FMT, b
From: |
Daniel Loffgren |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] fprintf-fn.h uses GCC_FMT, but doesn't include the definition of it |
Date: |
Mon, 25 Sep 2017 21:20:24 -0700 |
I am working on getting ppc-darwin-user working again, and this was one of the
many things preventing it from compiling. Your explanation sounds correct. I
believe it was the lack of osdep.h in the right .c files, since I added osdep.h
to the tops of all of the necessary files well after this change (I was fixing
things in order of compiler failures). I dropped this commit from my branch and
it didn’t break. So, feel free to disregard this.
> On Sep 25, 2017, at 9:18 AM, Eric Blake <address@hidden> wrote:
>
> On 09/24/2017 08:02 PM, Daniel Loffgren wrote:
>>
>> Signed-off-by: Daniel Loffgren <address@hidden>
>
> Hmm - you've identified a file with no listed maintainer. But
> qemu-trivial does seem like the right place to include it.
>
> Generally, we like patches to call out the topic that it is touching;
> also, the subject line should focus on the what, while the body focuses
> on the why. So a better commit message might be:
>
> maint: Make fprintf-fn.h self-contained
>
> Include the necessary headers so that GCC_FMT_ATTR is defined regardless
> of what client files use fprintf-fn.h.
>
>
> However, after saying that, I think your patch is not needed. Per
> HACKING, _all_ .c files must include osdep.h first, and osdep.h already
> includes compiler.h, therefore, any .c file that uses fprintf-fn.h
> already has GCC_FMT_ATTR in scope by the time it gets to the
> fprintf_function typedef. If you ran into a situation where you had a
> compile failure, please post more details of what failed for you, in
> case the problem was you forgetting to use osdep.h.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>