* [Qemu-trivial] [PATCH v7] block/vvfat: assert return value of fopen which may fail
@ 2014-08-18 8:00 ` zhanghailiang
0 siblings, 0 replies; 8+ messages in thread
From: zhanghailiang @ 2014-08-18 8:00 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, zhanghailiang, qemu-trivial, Li Liu, luonengjun,
peter.huangpeng, stefanha
From: Li Liu <john.liuli@huawei.com>
fopen() may return NULL which will cause setbuf() segmentfault
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Li Liu <john.liuli@huawei.com>
---
block/vvfat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/vvfat.c b/block/vvfat.c
index 70176b1..62023e1 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
DLOG(if (stderr == NULL) {
stderr = fopen("vvfat.log", "a");
+ assert(stderr);
setbuf(stderr, NULL);
})
--
1.7.12.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH v7] block/vvfat: assert return value of fopen which may fail
@ 2014-08-18 8:00 ` zhanghailiang
0 siblings, 0 replies; 8+ messages in thread
From: zhanghailiang @ 2014-08-18 8:00 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, zhanghailiang, qemu-trivial, Li Liu, luonengjun,
peter.huangpeng, stefanha
From: Li Liu <john.liuli@huawei.com>
fopen() may return NULL which will cause setbuf() segmentfault
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Li Liu <john.liuli@huawei.com>
---
block/vvfat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/vvfat.c b/block/vvfat.c
index 70176b1..62023e1 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
DLOG(if (stderr == NULL) {
stderr = fopen("vvfat.log", "a");
+ assert(stderr);
setbuf(stderr, NULL);
})
--
1.7.12.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH v7] block/vvfat: assert return value of fopen which may fail
2014-08-18 8:00 ` [Qemu-devel] " zhanghailiang
@ 2014-08-18 8:06 ` Peter Maydell
-1 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2014-08-18 8:06 UTC (permalink / raw)
To: zhanghailiang
Cc: Kevin Wolf, QEMU Trivial, Li Liu, Luonengjun, peter.huangpeng,
QEMU Developers, Stefan Hajnoczi
On 18 August 2014 09:00, zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
> From: Li Liu <john.liuli@huawei.com>
>
> fopen() may return NULL which will cause setbuf() segmentfault
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Li Liu <john.liuli@huawei.com>
> ---
> block/vvfat.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 70176b1..62023e1 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>
> DLOG(if (stderr == NULL) {
> stderr = fopen("vvfat.log", "a");
> + assert(stderr);
> setbuf(stderr, NULL);
> })
An assertion is no better than a segfault.
Better I think would be to just remove this whole lump of code
entirely. Lots of other files do debug printing to stderr without
attempting to open a file if stderr happens to be NULL, why
should vvfat.c be special?
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v7] block/vvfat: assert return value of fopen which may fail
@ 2014-08-18 8:06 ` Peter Maydell
0 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2014-08-18 8:06 UTC (permalink / raw)
To: zhanghailiang
Cc: Kevin Wolf, QEMU Trivial, Li Liu, Luonengjun, peter.huangpeng,
QEMU Developers, Stefan Hajnoczi
On 18 August 2014 09:00, zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
> From: Li Liu <john.liuli@huawei.com>
>
> fopen() may return NULL which will cause setbuf() segmentfault
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Li Liu <john.liuli@huawei.com>
> ---
> block/vvfat.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 70176b1..62023e1 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>
> DLOG(if (stderr == NULL) {
> stderr = fopen("vvfat.log", "a");
> + assert(stderr);
> setbuf(stderr, NULL);
> })
An assertion is no better than a segfault.
Better I think would be to just remove this whole lump of code
entirely. Lots of other files do debug printing to stderr without
attempting to open a file if stderr happens to be NULL, why
should vvfat.c be special?
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH v7] block/vvfat: assert return value of fopen which may fail
2014-08-18 8:06 ` Peter Maydell
@ 2014-08-18 11:42 ` Michael Tokarev
-1 siblings, 0 replies; 8+ messages in thread
From: Michael Tokarev @ 2014-08-18 11:42 UTC (permalink / raw)
To: Peter Maydell, zhanghailiang
Cc: Kevin Wolf, QEMU Trivial, Li Liu, Luonengjun, QEMU Developers,
peter.huangpeng, Stefan Hajnoczi
18.08.2014 12:06, Peter Maydell wrote:
> On 18 August 2014 09:00, zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
>> From: Li Liu <john.liuli@huawei.com>
>>
>> fopen() may return NULL which will cause setbuf() segmentfault
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Li Liu <john.liuli@huawei.com>
>> ---
>> block/vvfat.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index 70176b1..62023e1 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>>
>> DLOG(if (stderr == NULL) {
>> stderr = fopen("vvfat.log", "a");
>> + assert(stderr);
>> setbuf(stderr, NULL);
>> })
>
> An assertion is no better than a segfault.
>
> Better I think would be to just remove this whole lump of code
> entirely. Lots of other files do debug printing to stderr without
> attempting to open a file if stderr happens to be NULL, why
> should vvfat.c be special?
Indeed. I've applied a patch which just removes these 6 lines of code
(and sent it to the list too, for review).
Thanks,
/mjt
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH v7] block/vvfat: assert return value of fopen which may fail
@ 2014-08-18 11:42 ` Michael Tokarev
0 siblings, 0 replies; 8+ messages in thread
From: Michael Tokarev @ 2014-08-18 11:42 UTC (permalink / raw)
To: Peter Maydell, zhanghailiang
Cc: Kevin Wolf, QEMU Trivial, Li Liu, Luonengjun, QEMU Developers,
peter.huangpeng, Stefan Hajnoczi
18.08.2014 12:06, Peter Maydell wrote:
> On 18 August 2014 09:00, zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
>> From: Li Liu <john.liuli@huawei.com>
>>
>> fopen() may return NULL which will cause setbuf() segmentfault
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Li Liu <john.liuli@huawei.com>
>> ---
>> block/vvfat.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index 70176b1..62023e1 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>>
>> DLOG(if (stderr == NULL) {
>> stderr = fopen("vvfat.log", "a");
>> + assert(stderr);
>> setbuf(stderr, NULL);
>> })
>
> An assertion is no better than a segfault.
>
> Better I think would be to just remove this whole lump of code
> entirely. Lots of other files do debug printing to stderr without
> attempting to open a file if stderr happens to be NULL, why
> should vvfat.c be special?
Indeed. I've applied a patch which just removes these 6 lines of code
(and sent it to the list too, for review).
Thanks,
/mjt
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH v7] block/vvfat: assert return value of fopen which may fail
2014-08-18 11:42 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
@ 2014-08-19 6:28 ` zhanghailiang
-1 siblings, 0 replies; 8+ messages in thread
From: zhanghailiang @ 2014-08-19 6:28 UTC (permalink / raw)
To: Michael Tokarev
Cc: Kevin Wolf, Peter Maydell, QEMU Trivial, Li Liu, Luonengjun,
peter.huangpeng, QEMU Developers, Stefan Hajnoczi
On 2014/8/18 19:42, Michael Tokarev wrote:
> 18.08.2014 12:06, Peter Maydell wrote:
>> On 18 August 2014 09:00, zhanghailiang<zhang.zhanghailiang@huawei.com> wrote:
>>> From: Li Liu<john.liuli@huawei.com>
>>>
>>> fopen() may return NULL which will cause setbuf() segmentfault
>>>
>>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com>
>>> Signed-off-by: Li Liu<john.liuli@huawei.com>
>>> ---
>>> block/vvfat.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/block/vvfat.c b/block/vvfat.c
>>> index 70176b1..62023e1 100644
>>> --- a/block/vvfat.c
>>> +++ b/block/vvfat.c
>>> @@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>>>
>>> DLOG(if (stderr == NULL) {
>>> stderr = fopen("vvfat.log", "a");
>>> + assert(stderr);
>>> setbuf(stderr, NULL);
>>> })
>>
>> An assertion is no better than a segfault.
>>
>> Better I think would be to just remove this whole lump of code
>> entirely. Lots of other files do debug printing to stderr without
>> attempting to open a file if stderr happens to be NULL, why
>> should vvfat.c be special?
>
> Indeed. I've applied a patch which just removes these 6 lines of code
> (and sent it to the list too, for review).
>
> Thanks,
>
> /mjt
>
> .
>
OK, thanks.:)
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH v7] block/vvfat: assert return value of fopen which may fail
@ 2014-08-19 6:28 ` zhanghailiang
0 siblings, 0 replies; 8+ messages in thread
From: zhanghailiang @ 2014-08-19 6:28 UTC (permalink / raw)
To: Michael Tokarev
Cc: Kevin Wolf, Peter Maydell, QEMU Trivial, Li Liu, Luonengjun,
peter.huangpeng, QEMU Developers, Stefan Hajnoczi
On 2014/8/18 19:42, Michael Tokarev wrote:
> 18.08.2014 12:06, Peter Maydell wrote:
>> On 18 August 2014 09:00, zhanghailiang<zhang.zhanghailiang@huawei.com> wrote:
>>> From: Li Liu<john.liuli@huawei.com>
>>>
>>> fopen() may return NULL which will cause setbuf() segmentfault
>>>
>>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com>
>>> Signed-off-by: Li Liu<john.liuli@huawei.com>
>>> ---
>>> block/vvfat.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/block/vvfat.c b/block/vvfat.c
>>> index 70176b1..62023e1 100644
>>> --- a/block/vvfat.c
>>> +++ b/block/vvfat.c
>>> @@ -1084,6 +1084,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>>>
>>> DLOG(if (stderr == NULL) {
>>> stderr = fopen("vvfat.log", "a");
>>> + assert(stderr);
>>> setbuf(stderr, NULL);
>>> })
>>
>> An assertion is no better than a segfault.
>>
>> Better I think would be to just remove this whole lump of code
>> entirely. Lots of other files do debug printing to stderr without
>> attempting to open a file if stderr happens to be NULL, why
>> should vvfat.c be special?
>
> Indeed. I've applied a patch which just removes these 6 lines of code
> (and sent it to the list too, for review).
>
> Thanks,
>
> /mjt
>
> .
>
OK, thanks.:)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-08-19 6:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-18 8:00 [Qemu-trivial] [PATCH v7] block/vvfat: assert return value of fopen which may fail zhanghailiang
2014-08-18 8:00 ` [Qemu-devel] " zhanghailiang
2014-08-18 8:06 ` [Qemu-trivial] " Peter Maydell
2014-08-18 8:06 ` Peter Maydell
2014-08-18 11:42 ` [Qemu-trivial] " Michael Tokarev
2014-08-18 11:42 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2014-08-19 6:28 ` [Qemu-trivial] [Qemu-devel] " zhanghailiang
2014-08-19 6:28 ` [Qemu-devel] [Qemu-trivial] " zhanghailiang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.