[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message
From: |
Fabien Chouteau |
Subject: |
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message |
Date: |
Mon, 04 Feb 2013 12:27:05 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 |
On 02/04/2013 11:34 AM, Markus Armbruster wrote:
> Stefan Hajnoczi <address@hidden> writes:
>
>> On Fri, Feb 01, 2013 at 06:13:51PM +0100, Fabien Chouteau wrote:
>>>
>>> Signed-off-by: Fabien Chouteau <address@hidden>
>>> ---
>>> block.c | 13 ++++++++++---
>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> Hi Fabien,
>> Please always CC address@hidden 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