Re: Re: [PATCH v2 1/4] Include new arbitrary limits if not already defined

[Eric Blake]

On Mon, Jan 22, 2024 at 05:19:19PM +0000, Peter Maydell wrote:
> On Thu, 18 Jan 2024 at 16:03, Manolo de Medici <manolodemedici@gmail.com> 
> wrote:
> >
> > qemu uses the PATH_MAX and IOV_MAX constants extensively
> > in the code. Define these constants to sensible values ourselves
> > if the system doesn't define them already.
> >
> > Signed-off-by: Manolo de Medici <manolo.demedici@gmail.com>
> > ---
> >  include/qemu/osdep.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index 9a405bed89..9fb6ac5c64 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -363,6 +363,14 @@ void QEMU_ERROR("code path is reachable")
> >  #define TIME_MAX TYPE_MAXIMUM(time_t)
> >  #endif
> >
> > +#ifndef PATH_MAX
> > +#define PATH_MAX 1024
> > +#endif

POSIX requires that _XOPEN_PATH_MAX be defined as 1024, as a bare
minimum for any system implementing X/Open extensions to POSIX, so
this number is reasonable.  It is also small enough that most of our
uses where PATH_MAX is used for stack allocation (rather than heap
allocation) don't explode.  But the /reason/ that GNU Hurd refuses to
define PATH_MAX is /because/ it is an arbitrary limit, and GNU Hurd
goes out of its way to not impose such a small limit on the user.  The
intent is that portable code should be written to malloc() any path
operation in order to deal with ANY size file name thrown at the code,
rather than stack-allocate and risk truncation when the limits chosen
were too small for the user's desires.

I'm not opposed to this patch with a stronger commit message, but the
commit message would do well to show that an attempt was made to audit
all existing uses of PATH_MAX and why we still need them rather than
malloc()ing.  A stronger patch would be one that eliminates the use of
PATH_MAX from the code base, on the grounds that truly portable code
can handle all pathnames that the underlying system already has memory
on hand to throw at the program; note that I am /not/ insisting on
such a stronger code guarantee, but merely that we document our design
decision of why we are unable/unwilling to go that far if the stronger
guarantee turns out to be impractical.

> > +
> > +#ifndef IOV_MAX
> > +#define IOV_MAX 1024
> > +#endif

Here, POSIX only requires _XOPEN_IOV_MAX to be 16 (if you're going to
do sharded scatter-gather I/O, portable code has to cater to systems
that do not tolerate you trying to cram more than 16 shards into one
syscall).  Older Solaris actually had a limit this low, but I can
easily test that Linux has a limit of 1024, and a Google search seems
to concur that the various BSDs have also settled on 1024.  GNU Hurd
obviously supports more than 1024, but capping at this number is
reasonable.  Unlike eradicating PATH_MAX from the code base, this is
one that makes more sense to me to have in place; although it would
still be worth documenting that all known systems that qemu targets
(including GNU Hurd, if your intent is to make that such a system)
support 1024 rather than the smaller minimum of 16 that POSIX
mandates.

> > +
> >  /* Mac OSX has a <stdint.h> bug that incorrectly defines SIZE_MAX with
> >   * the wrong type. Our replacement isn't usable in preprocessor
> >   * expressions, but it is sufficient for our needs. */
> 
> Ccing some people who know more about portability concerns
> than I do...
> 
> thanks
> -- PMM
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org