Re: [PATCH v2] 9p: move P9_XATTR_SIZE_MAX from 9p.h to 9p.c

[Thomas Huth <thuth@redhat.com>]

On 31/03/2022 22.06, Will Cohen wrote:
> On Thu, Mar 31, 2022 at 4:00 PM Peter Maydell <peter.maydell@linaro.org 
> <mailto:peter.maydell@linaro.org>> wrote:
> 
>     On Thu, 31 Mar 2022 at 19:27, Will Cohen <wwcohen@gmail.com
>     <mailto:wwcohen@gmail.com>> wrote:
>      >
>      > The patch set adding 9p functionality to darwin introduced an issue
>      > where limits.h, which defines XATTR_SIZE_MAX, is included in 9p.c,
>      > though the referenced constant is needed in 9p.h. This commit fixes that
>      > issue by moving the definition of P9_XATTR_SIZE_MAX, which uses
>      > XATTR_SIZE_MAX, to also be in 9p.c.
>      >
>      > Additionally, this commit moves the location of the system headers
>      > include in 9p.c to occur before the project headers.
>      >
>      > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/950
>     <https://gitlab.com/qemu-project/qemu/-/issues/950>
>      > Fixes: 38d7fd68b0 ("9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX")
>      >
>      > Signed-off-by: Will Cohen <wwcohen@gmail.com <mailto:wwcohen@gmail.com>>
>      > ---
>      >  hw/9pfs/9p.c | 28 +++++++++++++++++++++++-----
>      >  hw/9pfs/9p.h | 18 ------------------
>      >  2 files changed, 23 insertions(+), 23 deletions(-)
>      >
>      > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>      > index dcaa602d4c..b9152c7882 100644
>      > --- a/hw/9pfs/9p.c
>      > +++ b/hw/9pfs/9p.c
>      > @@ -16,6 +16,11 @@
>      >   * https://wiki.qemu.org/Documentation/9p
>     <https://wiki.qemu.org/Documentation/9p>
>      >   */
>      >
>      > +#ifdef CONFIG_LINUX
>      > +#include <linux/limits.h>
>      > +#else
>      > +#include <limits.h>
>      > +#endif
>      >  #include "qemu/osdep.h"
> 
>     osdep.h must always be the first include line in any .c file.
> 
> Understood, apologies -- if there's other changes for a v3 I can resubmit 
> accordingly, but if this otherwise looks okay then I would be fine with a 
> QEMU maintainer adjusting the header placement as needed when preparing for 
> submission to the main tree.

Makes sense. I'm currently assembling a pull req with some misc fixes for 
7.0 ... if Christian & Greg do not have any other patches pending right now, 
I could throw this in, with the osdep.h location fixed.

  Thomas