* Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.
@ 2014-08-03 1:14 ` Peter Crosthwaite
0 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-08-03 1:14 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, qemu-devel@nongnu.org Developers
On Sat, Aug 2, 2014 at 11:26 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 31.07.2014 04:31, Peter Crosthwaite wrote:
>> To avoid callsites with optional output having to NULL guard.
>
> Isn't it a bit backwards? If we don't need output, maybe we should
> not call hexdump in the first place?
>
Well my thinking is a NULL File * is a good way to "don't need output"
and then it means then the multiple callsites can avoid:
if (fp) {
Note that many of the exitinst
> Thanks,
>
> /mjt
>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> Noting in-tree is affected by this yet, but I though I'd get this
>> out of the way straight-up rather than elongate other series.
>>
>> util/hexdump.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/util/hexdump.c b/util/hexdump.c
>> index 969b340..b607236 100644
>> --- a/util/hexdump.c
>> +++ b/util/hexdump.c
>> @@ -19,6 +19,9 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>> {
>> unsigned int b;
>>
>> + if (!fp) {
>> + return;
>> + }
>> for (b = 0; b < size; b++) {
>> if ((b % 16) == 0) {
>> fprintf(fp, "%s: %04x:", prefix, b);
>>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.
2014-08-03 1:14 ` [Qemu-devel] [Qemu-trivial] " Peter Crosthwaite
@ 2014-08-03 1:15 ` Peter Crosthwaite
-1 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-08-03 1:15 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, qemu-devel@nongnu.org Developers
Sorry premature send (I guess I just discovered a new keyboard shortcut!)
On Sun, Aug 3, 2014 at 11:14 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Aug 2, 2014 at 11:26 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 31.07.2014 04:31, Peter Crosthwaite wrote:
>>> To avoid callsites with optional output having to NULL guard.
>>
>> Isn't it a bit backwards? If we don't need output, maybe we should
>> not call hexdump in the first place?
>>
>
> Well my thinking is a NULL File * is a good way to "don't need output"
> and then it means then the multiple callsites can avoid:
>
>
> if (fp) {
>
>
> Note that many of the exitinst
>
>
>
>> Thanks,
>>
>> /mjt
>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>> Noting in-tree is affected by this yet, but I though I'd get this
>>> out of the way straight-up rather than elongate other series.
>>>
>>> util/hexdump.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/util/hexdump.c b/util/hexdump.c
>>> index 969b340..b607236 100644
>>> --- a/util/hexdump.c
>>> +++ b/util/hexdump.c
>>> @@ -19,6 +19,9 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>>> {
>>> unsigned int b;
>>>
>>> + if (!fp) {
>>> + return;
>>> + }
>>> for (b = 0; b < size; b++) {
>>> if ((b % 16) == 0) {
>>> fprintf(fp, "%s: %04x:", prefix, b);
>>>
>>
>>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.
@ 2014-08-03 1:15 ` Peter Crosthwaite
0 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-08-03 1:15 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, qemu-devel@nongnu.org Developers
Sorry premature send (I guess I just discovered a new keyboard shortcut!)
On Sun, Aug 3, 2014 at 11:14 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Aug 2, 2014 at 11:26 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 31.07.2014 04:31, Peter Crosthwaite wrote:
>>> To avoid callsites with optional output having to NULL guard.
>>
>> Isn't it a bit backwards? If we don't need output, maybe we should
>> not call hexdump in the first place?
>>
>
> Well my thinking is a NULL File * is a good way to "don't need output"
> and then it means then the multiple callsites can avoid:
>
>
> if (fp) {
>
>
> Note that many of the exitinst
>
>
>
>> Thanks,
>>
>> /mjt
>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>> Noting in-tree is affected by this yet, but I though I'd get this
>>> out of the way straight-up rather than elongate other series.
>>>
>>> util/hexdump.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/util/hexdump.c b/util/hexdump.c
>>> index 969b340..b607236 100644
>>> --- a/util/hexdump.c
>>> +++ b/util/hexdump.c
>>> @@ -19,6 +19,9 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>>> {
>>> unsigned int b;
>>>
>>> + if (!fp) {
>>> + return;
>>> + }
>>> for (b = 0; b < size; b++) {
>>> if ((b % 16) == 0) {
>>> fprintf(fp, "%s: %04x:", prefix, b);
>>>
>>
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.
2014-08-03 1:14 ` [Qemu-devel] [Qemu-trivial] " Peter Crosthwaite
@ 2014-08-03 1:17 ` Peter Crosthwaite
-1 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-08-03 1:17 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, qemu-devel@nongnu.org Developers
On Sun, Aug 3, 2014 at 11:14 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Aug 2, 2014 at 11:26 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 31.07.2014 04:31, Peter Crosthwaite wrote:
>>> To avoid callsites with optional output having to NULL guard.
>>
>> Isn't it a bit backwards? If we don't need output, maybe we should
>> not call hexdump in the first place?
>>
>
Well my thinking is a NULL File * is a good way to "don't need output"
and then it means then the multiple callsites can avoid:
if (fp) {
qemu_hexdump(foo, fp, ..)
}
Note that many of the existing call this with stderr as FP argument
which in theory should be change to something that backs onto
qemu_log. I suspect the file handle in this case will be NULL when
logging is disabled and this patch avoid having to add all these
conditionals.
Regards,
Peter
>> Thanks,
>>
>> /mjt
>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>> Noting in-tree is affected by this yet, but I though I'd get this
>>> out of the way straight-up rather than elongate other series.
>>>
>>> util/hexdump.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/util/hexdump.c b/util/hexdump.c
>>> index 969b340..b607236 100644
>>> --- a/util/hexdump.c
>>> +++ b/util/hexdump.c
>>> @@ -19,6 +19,9 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>>> {
>>> unsigned int b;
>>>
>>> + if (!fp) {
>>> + return;
>>> + }
>>> for (b = 0; b < size; b++) {
>>> if ((b % 16) == 0) {
>>> fprintf(fp, "%s: %04x:", prefix, b);
>>>
>>
>>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.
@ 2014-08-03 1:17 ` Peter Crosthwaite
0 siblings, 0 replies; 12+ messages in thread
From: Peter Crosthwaite @ 2014-08-03 1:17 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-trivial, qemu-devel@nongnu.org Developers
On Sun, Aug 3, 2014 at 11:14 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Aug 2, 2014 at 11:26 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 31.07.2014 04:31, Peter Crosthwaite wrote:
>>> To avoid callsites with optional output having to NULL guard.
>>
>> Isn't it a bit backwards? If we don't need output, maybe we should
>> not call hexdump in the first place?
>>
>
Well my thinking is a NULL File * is a good way to "don't need output"
and then it means then the multiple callsites can avoid:
if (fp) {
qemu_hexdump(foo, fp, ..)
}
Note that many of the existing call this with stderr as FP argument
which in theory should be change to something that backs onto
qemu_log. I suspect the file handle in this case will be NULL when
logging is disabled and this patch avoid having to add all these
conditionals.
Regards,
Peter
>> Thanks,
>>
>> /mjt
>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>> Noting in-tree is affected by this yet, but I though I'd get this
>>> out of the way straight-up rather than elongate other series.
>>>
>>> util/hexdump.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/util/hexdump.c b/util/hexdump.c
>>> index 969b340..b607236 100644
>>> --- a/util/hexdump.c
>>> +++ b/util/hexdump.c
>>> @@ -19,6 +19,9 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size)
>>> {
>>> unsigned int b;
>>>
>>> + if (!fp) {
>>> + return;
>>> + }
>>> for (b = 0; b < size; b++) {
>>> if ((b % 16) == 0) {
>>> fprintf(fp, "%s: %04x:", prefix, b);
>>>
>>
>>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.
2014-08-03 1:17 ` [Qemu-devel] [Qemu-trivial] " Peter Crosthwaite
@ 2014-08-03 13:11 ` Peter Maydell
-1 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2014-08-03 13:11 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: qemu-trivial, Michael Tokarev, qemu-devel@nongnu.org Developers
On 3 August 2014 02:17, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> Note that many of the existing call this with stderr as FP argument
> which in theory should be change to something that backs onto
> qemu_log. I suspect the file handle in this case will be NULL when
> logging is disabled and this patch avoid having to add all these
> conditionals.
For the qemu_log case we should handle the "don't log" condition
in the qemu_log() wrapper, especially since we probably want the
wrapper to take a log-mask anyway.
thanks
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.
@ 2014-08-03 13:11 ` Peter Maydell
0 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2014-08-03 13:11 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: qemu-trivial, Michael Tokarev, qemu-devel@nongnu.org Developers
On 3 August 2014 02:17, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> Note that many of the existing call this with stderr as FP argument
> which in theory should be change to something that backs onto
> qemu_log. I suspect the file handle in this case will be NULL when
> logging is disabled and this patch avoid having to add all these
> conditionals.
For the qemu_log case we should handle the "don't log" condition
in the qemu_log() wrapper, especially since we probably want the
wrapper to take a log-mask anyway.
thanks
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread