[Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

To avoid callsites with optional output having to NULL guard.

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);
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

To avoid callsites with optional output having to NULL guard.

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);
-- 
2.0.1.1.gfbfc394

Re: [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Michael Tokarev]

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?

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);
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Michael Tokarev]

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?

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);
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

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);
>>
>
>

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

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);
>>
>
>

Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

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);
>>>
>>
>>

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

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);
>>>
>>
>>

Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

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);
>>>
>>
>>

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Crosthwaite]

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);
>>>
>>
>>

Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Maydell]

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

Re: [Qemu-devel] [Qemu-trivial] [PATCH v1 1/1] hexdump: Add null guard on output file.

[Peter Maydell]

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