Re: [Qemu-trivial] [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message

[Fabien Chouteau <chouteau@adacore.com>]

On 02/04/2013 11:34 AM, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
>> On Fri, Feb 01, 2013 at 06:13:51PM +0100, Fabien Chouteau wrote:
>>>
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>> ---
>>>  block.c |   13 ++++++++++---
>>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> Hi Fabien,
>> Please always CC qemu-devel@nongnu.org.  All patches must be on
>> qemu-devel so that the community can review them - not everyone
>> subscribes to qemu-trivial.
>>
>> Thanks,
>> Stefan
>>
>>> diff --git a/block.c b/block.c
>>> index ba67c0d..3bf8eda 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -428,9 +428,16 @@ int get_tmp_filename(char *filename, int size)
>>>      /* 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());
>>> +    if (GetTempPath(MAX_PATH, temp_dir) == 0) {
>>> +        fprintf(stderr, "GetTempPath() error: %d\n", GetLastError());
>>> +        return -GetLastError();
>>> +    }
>>> +    if (GetTempFileName(temp_dir, "qem", 0, filename) == 0) {
>>> +        fprintf(stderr, "GetTempFileName(%s) error: %d\n", temp_dir,
>>> +                GetLastError());
>>> +        return -GetLastError();
>>> +    }
>>> +    return 0;
>>>  #else
>>>      int fd;
>>>      const char *tmpdir;
>
> get_tmp_filename() is not supposed to print to stderr, that's the
> caller's job.
>

Why? The caller doesn't know the difference between Windows/Linux
implementation. And the error handling would have to be duplicated.

It's not the first time I add error output in Windows code. Specially in
block/, when there's an error, the only thing you get is: "operation not
permitted". It's not very helpful and you have to dig in the code to
find which function failed.

-- 
Fabien Chouteau