Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands

[Eric Blake]

On 02/24/2012 01:26 PM, Luiz Capitulino wrote:
> On Fri, 24 Feb 2012 12:40:17 -0700
> Eric Blake <address@hidden> wrote:
> 
>> On 02/24/2012 12:01 PM, Luiz Capitulino wrote:
>>
>>>> +    BlockDriver *drv;
>>>> +    int i, j, escape;
>>>> +    char new_filename[2048], *filename;
>>>
>>> I'd use PATH_MAX for new_filename's size.
>>
>> PATH_MAX need not be defined (and on Hurd, it intentionally is not
>> defined); or might be so huge as to be useless.
> 
> Aren't those extreme cases? PATH_MAX is a standard define and is used in
> QEMU in several places. If it's not good here, it shouldn't be good anywhere.

PATH_MAX is specifically declared in POSIX to be defined if there is a
limit, or undefined if there is no limit.  There is no limit in GNU
Hurd, so PATH_MAX is undefined there, and you will get a compile error
(then again, no one has ported qemu to Hurd).

Here's what gnulib has recommended:

https://lists.gnu.org/archive/html/bug-gnulib/2011-06/msg00328.html

> A package like coreutils can also do
>   #ifndef PATH_MAX
>   # define PATH_MAX 8192
>   #endif
> in its system.h.
> 
> Looking at both uses of PATH_MAX in coreutils (src/pwd.c:88 and
> src/remove.c:186) the value of PATH_MAX is capped by 8192 or 16384 anyway.
> So, on systems like GNU/Hurd, where filenames can have arbitrary size, you
> are calling pathconf for no real purpose.
> 
> To me, this confirms that a generic pathmax.h (like the one in gnulib)
> should only define PATH_MAX when it makes sense - like POSIX says -,
> and that the handling of the GNU/Hurd case should be done on a case-by-case
> basis:
>   - Either a package-wide handling, or a per-file handling.
>   - Either a fallback value of 8192, or a fallback value of
>     pathconf ("/", _PC_PATH_MAX), or just a #ifdef test.

Other mails in that thread are also an interesting read.

In short, use of PATH_MAX should only ever be used to optimize routines
to the common case; in which case, you can pick your own cap for
PATH_MAX if the implementation did not provide one or reduce the
implementation's 8k PATH_MAX down to something like 2048 that you can
safely fit on the stack for the common case before malloc'ing for the
larger strings.  But using it as a bounds to a statically-sized object
is a recipe for artificially limiting software; if you are okay with
introducing that artificial limit, then go for it; but if you want to be
truly portable, it is best to never use PATH_MAX as an array bounds, and
to write fallback code paths to handle the cases where user input
exceeds PATH_MAX but can still be handled without error by the system
you are running on.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature