[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH] block: Fix regression for MinGW (assertion ca
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-trivial] [PATCH] block: Fix regression for MinGW (assertion caused by short string) |
Date: |
Mon, 19 Nov 2012 12:31:11 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sun, Nov 18, 2012 at 09:26:15AM +0100, Stefan Weil wrote:
> The local string tmp_filename is passed to function get_tmp_filename
> which expects a string with minimum size MAX_PATH for w32 hosts.
>
> MAX_PATH is 260 and PATH_MAX is 259, so tmp_filename was too short.
>
> Commit eba25057b9a5e19d10ace2bc7716667a31297169 introduced this
> regression.
>
> Signed-off-by: Stefan Weil <address@hidden>
> ---
> block.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index 49dd6bb..8739635 100644
> --- a/block.c
> +++ b/block.c
> @@ -787,7 +787,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
> int flags,
> BlockDriver *drv)
> {
> int ret;
> - char tmp_filename[PATH_MAX];
> + char tmp_filename[PATH_MAX + 1];
This is not maintainable - the patch is making an assumption about the relative
values of MAX_PATH and PATH_MAX. There is no comment explaining this so it's
likely to be changed and break in the future again.
A clean solution is to change get_tmp_filename() so the caller doesn't need to
pass in a fixed size:
/*
* Create a uniquely-named empty temporary file.
* Return 0 upon success, otherwise a negative errno value.
*
* The filename must be freed with g_free() when it is no longer needed.
*/
int get_tmp_filename(char **filename)
The existing get_tmp_filename() code has another problem. Here is the Windows
get_tmp_filename() code:
char temp_dir[MAX_PATH];
/* GetTempFileName requires that its output buffer (4th param)
have length MAX_PATH or greater. */
assert(size >= MAX_PATH);
return (GetTempPath(MAX_PATH, temp_dir)
&& GetTempFileName(temp_dir, "qem", 0, filename)
? 0 : -GetLastError());
The assert has an off-by-one error because the documentation says:
"This buffer should be MAX_PATH characters to accommodate the path plus the
terminating null character."
http://msdn.microsoft.com/en-us/library/windows/desktop/aa364991(v=vs.85).aspx
Since the function returns -errno the assert could be turned into:
/* GetTempFileName requires that its output buffer (4th param)
have length MAX_PATH + 1 or greater. */
if (size < MAX_PATH + 1) {
return -ENOSPC;
}
Stefan