Stefan Weil wrote:
> Am 30.05.2012 09:46, schrieb Jim Meyering:
>> From: Jim Meyering<[email protected]>
>>
>> Also, use PATH_MAX, rather than the arbitrary 1024.
>> Using PATH_MAX is more consistent with other filename-related
>> variables in this file, like backing_filename and tmp_filename.
>>
>> Acked-by: Kevin Wolf<[email protected]>
>> Signed-off-by: Jim Meyering<[email protected]>
>> ---
>> block.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index af2ab4f..efc7071 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1232,7 +1232,7 @@ int bdrv_commit(BlockDriverState *bs)
>> int n, ro, open_flags;
>> int ret = 0, rw_ret = 0;
>> uint8_t *buf;
>> - char filename[1024];
>> + char filename[PATH_MAX];
>> BlockDriverState *bs_rw, *bs_ro;
>>
>> if (!drv)
>> @@ -1252,7 +1252,8 @@ int bdrv_commit(BlockDriverState *bs)
>>
>> backing_drv = bs->backing_hd->drv;
>> ro = bs->backing_hd->read_only;
>> - strncpy(filename, bs->backing_hd->filename, sizeof(filename));
>> + /* Use pstrcpy (not strncpy): filename must be NUL-terminated. */
>> + pstrcpy(filename, sizeof(filename), bs->backing_hd->filename);
>> open_flags = bs->backing_hd->open_flags;
>>
>> if (ro) {
>
> PATH_MAX can have any value, from 259 or 512 on Windows to
> 4096 on Linux or even more.
We've seen PATH_MAX values of 32KiB, and even INT_MAX.
On the Hurd it wasn't defined at all for many years --
that may still be the case. It's enough of a portability
rats nest that we've removed all non-advisory uses from the
100+ programs in the coreutils package.
I'm 100% with you that PATH_MAX is best avoided, but it's certainly
an improvement over a hard-coded constant like 1024 -- who knows
if/when it may mistakenly used as an array dimension supposedly
adequate to hold a PATH_MAX=4096-length buffer.
As the commit log comment suggests, I'd prefer consistency first
(at least here), with a larger scope clean-up effort to do something
about all of these:
$ git grep -E '\[(4096|2048|1024|512)\]'|wc -l
135
But if you'd prefer, I'm happy to revert that part of the above change.
It certainly shouldn't hold up the buffer overrun fix.
> I usually avoid arrays with more than some hundred bytes on the stack.
> Even if the stack is large enough, it's not good for caching.
>
> Of course, avoiding arbitrary limits like PATH_MAX would be best.
>
> Using a QEMU_PATH_MAX which is the same for all hosts might be
> the second best solution.
