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

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Freitag, 1. April 2022 13:01:31 CEST Thomas Huth wrote:
> 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

That would be appreciated, nothing else for 7.0 at this point. Thanks Thomas!

Best regards,
Christian Schoenebeck