[PATCH] hw/display/omap_dss: Replace fprintf() call by qemu_log_mask(LOG_UNIMP)

Philippe Mathieu-Daudé posted 1 patch 3 years, 11 months ago
Test docker-mingw@fedora passed
Test checkpatch passed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200524103833.8188-1-f4bug@amsat.org
hw/display/omap_dss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] hw/display/omap_dss: Replace fprintf() call by qemu_log_mask(LOG_UNIMP)
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
Replace fprintf() call by qemu_log_mask(LOG_UNIMP), which is
disabled by default. This avoid flooding the terminal when
fuzzing the device.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/display/omap_dss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/omap_dss.c b/hw/display/omap_dss.c
index 32dc0d6aa7..21fde58a26 100644
--- a/hw/display/omap_dss.c
+++ b/hw/display/omap_dss.c
@@ -619,7 +619,7 @@ static void omap_rfbi_transfer_start(struct omap_dss_s *s)
     if (s->rfbi.control & (1 << 1)) {				/* BYPASS */
         /* TODO: in non-Bypass mode we probably need to just assert the
          * DRQ and wait for DMA to write the pixels.  */
-        fprintf(stderr, "%s: Bypass mode unimplemented\n", __func__);
+        qemu_log_mask(LOG_UNIMP, "%s: Bypass mode unimplemented\n", __func__);
         return;
     }
 
-- 
2.21.3


Re: [PATCH] hw/display/omap_dss: Replace fprintf() call by qemu_log_mask(LOG_UNIMP)
Posted by Alexander Bulekov 3 years, 11 months ago
On 200524 1238, Philippe Mathieu-Daudé wrote:
> Replace fprintf() call by qemu_log_mask(LOG_UNIMP), which is
> disabled by default. This avoid flooding the terminal when
> fuzzing the device.
> 
Hi Philippe,
libfuzzer also lets you pass -close_fd_mask=[1,2,3] to close stdout,
stderr or both. As a side-effect, In this mode, libfuzzer will write the
crashing stacktraces to individual files ./.$PID, instead of to the
terminal's output. As far as I know, closing the FDs doesn't bypass all
of printf's allocation/formatting code, so getting rid of calls can
still be a good idea for performance, sometimes.
-Alex

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/display/omap_dss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/omap_dss.c b/hw/display/omap_dss.c
> index 32dc0d6aa7..21fde58a26 100644
> --- a/hw/display/omap_dss.c
> +++ b/hw/display/omap_dss.c
> @@ -619,7 +619,7 @@ static void omap_rfbi_transfer_start(struct omap_dss_s *s)
>      if (s->rfbi.control & (1 << 1)) {				/* BYPASS */
>          /* TODO: in non-Bypass mode we probably need to just assert the
>           * DRQ and wait for DMA to write the pixels.  */
> -        fprintf(stderr, "%s: Bypass mode unimplemented\n", __func__);
> +        qemu_log_mask(LOG_UNIMP, "%s: Bypass mode unimplemented\n", __func__);
>          return;
>      }
>  
> -- 
> 2.21.3
> 

Re: [PATCH] hw/display/omap_dss: Replace fprintf() call by qemu_log_mask(LOG_UNIMP)
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 5/24/20 2:41 PM, Alexander Bulekov wrote:
> On 200524 1238, Philippe Mathieu-Daudé wrote:
>> Replace fprintf() call by qemu_log_mask(LOG_UNIMP), which is
>> disabled by default. This avoid flooding the terminal when
>> fuzzing the device.
>>
> Hi Philippe,
> libfuzzer also lets you pass -close_fd_mask=[1,2,3] to close stdout,
> stderr or both. As a side-effect, In this mode, libfuzzer will write the
> crashing stacktraces to individual files ./.$PID, instead of to the
> terminal's output.

Thanks, good to know.

This information is worth adding in docs/devel/fuzzing.txt :P

> As far as I know, closing the FDs doesn't bypass all
> of printf's allocation/formatting code, so getting rid of calls can
> still be a good idea for performance, sometimes.

Certainly.

> -Alex
> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/display/omap_dss.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/display/omap_dss.c b/hw/display/omap_dss.c
>> index 32dc0d6aa7..21fde58a26 100644
>> --- a/hw/display/omap_dss.c
>> +++ b/hw/display/omap_dss.c
>> @@ -619,7 +619,7 @@ static void omap_rfbi_transfer_start(struct omap_dss_s *s)
>>      if (s->rfbi.control & (1 << 1)) {				/* BYPASS */
>>          /* TODO: in non-Bypass mode we probably need to just assert the
>>           * DRQ and wait for DMA to write the pixels.  */
>> -        fprintf(stderr, "%s: Bypass mode unimplemented\n", __func__);
>> +        qemu_log_mask(LOG_UNIMP, "%s: Bypass mode unimplemented\n", __func__);
>>          return;
>>      }
>>  
>> -- 
>> 2.21.3
>>
> 

Re: [PATCH] hw/display/omap_dss: Replace fprintf() call by qemu_log_mask(LOG_UNIMP)
Posted by Thomas Huth 3 years, 10 months ago
On 24/05/2020 12.38, Philippe Mathieu-Daudé wrote:
> Replace fprintf() call by qemu_log_mask(LOG_UNIMP), which is
> disabled by default. This avoid flooding the terminal when
> fuzzing the device.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/display/omap_dss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/omap_dss.c b/hw/display/omap_dss.c
> index 32dc0d6aa7..21fde58a26 100644
> --- a/hw/display/omap_dss.c
> +++ b/hw/display/omap_dss.c
> @@ -619,7 +619,7 @@ static void omap_rfbi_transfer_start(struct omap_dss_s *s)
>      if (s->rfbi.control & (1 << 1)) {				/* BYPASS */
>          /* TODO: in non-Bypass mode we probably need to just assert the
>           * DRQ and wait for DMA to write the pixels.  */
> -        fprintf(stderr, "%s: Bypass mode unimplemented\n", __func__);
> +        qemu_log_mask(LOG_UNIMP, "%s: Bypass mode unimplemented\n", __func__);
>          return;
>      }

Reviewed-by: Thomas Huth <thuth@redhat.com>