All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/2] Constify some variable
@ 2015-06-11 13:17 Frediano Ziglio
  2015-06-11 13:17 ` [Qemu-devel] [PATCH 2/2] Check value for invalid negative values Frediano Ziglio
  2015-06-17 19:23 ` [Qemu-devel] [PATCH 1/2] Constify some variable Michael Tokarev
  0 siblings, 2 replies; 8+ messages in thread
From: Frediano Ziglio @ 2015-06-11 13:17 UTC (permalink / raw)
  To: fziglio, Gerd Hoffmann; +Cc: qemu-trivial, qemu-devel

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 hw/display/qxl-logger.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
index c900c2c..d944d3f 100644
--- a/hw/display/qxl-logger.c
+++ b/hw/display/qxl-logger.c
@@ -22,7 +22,7 @@
 #include "qemu/timer.h"
 #include "qxl.h"
 
-static const char *qxl_type[] = {
+static const char *const qxl_type[] = {
     [ QXL_CMD_NOP ]     = "nop",
     [ QXL_CMD_DRAW ]    = "draw",
     [ QXL_CMD_UPDATE ]  = "update",
@@ -31,7 +31,7 @@ static const char *qxl_type[] = {
     [ QXL_CMD_SURFACE ] = "surface",
 };
 
-static const char *qxl_draw_type[] = {
+static const char *const qxl_draw_type[] = {
     [ QXL_DRAW_NOP         ] = "nop",
     [ QXL_DRAW_FILL        ] = "fill",
     [ QXL_DRAW_OPAQUE      ] = "opaque",
@@ -48,7 +48,7 @@ static const char *qxl_draw_type[] = {
     [ QXL_DRAW_ALPHA_BLEND ] = "alpha-blend",
 };
 
-static const char *qxl_draw_effect[] = {
+static const char *const qxl_draw_effect[] = {
     [ QXL_EFFECT_BLEND            ] = "blend",
     [ QXL_EFFECT_OPAQUE           ] = "opaque",
     [ QXL_EFFECT_REVERT_ON_DUP    ] = "revert-on-dup",
@@ -59,12 +59,12 @@ static const char *qxl_draw_effect[] = {
     [ QXL_EFFECT_OPAQUE_BRUSH     ] = "opaque-brush",
 };
 
-static const char *qxl_surface_cmd[] = {
+static const char *const qxl_surface_cmd[] = {
    [ QXL_SURFACE_CMD_CREATE  ] = "create",
    [ QXL_SURFACE_CMD_DESTROY ] = "destroy",
 };
 
-static const char *spice_surface_fmt[] = {
+static const char *const spice_surface_fmt[] = {
    [ SPICE_SURFACE_FMT_INVALID  ] = "invalid",
    [ SPICE_SURFACE_FMT_1_A      ] = "alpha/1",
    [ SPICE_SURFACE_FMT_8_A      ] = "alpha/8",
@@ -74,14 +74,14 @@ static const char *spice_surface_fmt[] = {
    [ SPICE_SURFACE_FMT_32_ARGB  ] = "ARGB/32",
 };
 
-static const char *qxl_cursor_cmd[] = {
+static const char *const qxl_cursor_cmd[] = {
    [ QXL_CURSOR_SET   ] = "set",
    [ QXL_CURSOR_MOVE  ] = "move",
    [ QXL_CURSOR_HIDE  ] = "hide",
    [ QXL_CURSOR_TRAIL ] = "trail",
 };
 
-static const char *spice_cursor_type[] = {
+static const char *const spice_cursor_type[] = {
    [ SPICE_CURSOR_TYPE_ALPHA   ] = "alpha",
    [ SPICE_CURSOR_TYPE_MONO    ] = "mono",
    [ SPICE_CURSOR_TYPE_COLOR4  ] = "color4",
@@ -91,7 +91,7 @@ static const char *spice_cursor_type[] = {
    [ SPICE_CURSOR_TYPE_COLOR32 ] = "color32",
 };
 
-static const char *qxl_v2n(const char *n[], size_t l, int v)
+static const char *qxl_v2n(const char *const n[], size_t l, int v)
 {
     if (v >= l || !n[v]) {
         return "???";
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/2] Check value for invalid negative values
  2015-06-11 13:17 [Qemu-devel] [PATCH 1/2] Constify some variable Frediano Ziglio
@ 2015-06-11 13:17 ` Frediano Ziglio
  2015-06-17 19:27   ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
  2015-06-17 19:23 ` [Qemu-devel] [PATCH 1/2] Constify some variable Michael Tokarev
  1 sibling, 1 reply; 8+ messages in thread
From: Frediano Ziglio @ 2015-06-11 13:17 UTC (permalink / raw)
  To: fziglio, Gerd Hoffmann; +Cc: qemu-trivial, qemu-devel

In qxl_v2n check that value is not negative.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 hw/display/qxl-logger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
index d944d3f..faed869 100644
--- a/hw/display/qxl-logger.c
+++ b/hw/display/qxl-logger.c
@@ -93,7 +93,7 @@ static const char *const spice_cursor_type[] = {
 
 static const char *qxl_v2n(const char *const n[], size_t l, int v)
 {
-    if (v >= l || !n[v]) {
+    if (v < 0 || v >= l || !n[v]) {
         return "???";
     }
     return n[v];
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Constify some variable
  2015-06-11 13:17 [Qemu-devel] [PATCH 1/2] Constify some variable Frediano Ziglio
  2015-06-11 13:17 ` [Qemu-devel] [PATCH 2/2] Check value for invalid negative values Frediano Ziglio
@ 2015-06-17 19:23 ` Michael Tokarev
  1 sibling, 0 replies; 8+ messages in thread
From: Michael Tokarev @ 2015-06-17 19:23 UTC (permalink / raw)
  To: Frediano Ziglio, Gerd Hoffmann; +Cc: qemu-trivial, qemu-devel

11.06.2015 16:17, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  hw/display/qxl-logger.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
> index c900c2c..d944d3f 100644
> --- a/hw/display/qxl-logger.c
> +++ b/hw/display/qxl-logger.c
> @@ -22,7 +22,7 @@
>  #include "qemu/timer.h"
>  #include "qxl.h"
>  
> -static const char *qxl_type[] = {
> +static const char *const qxl_type[] = {
...

Applied to trivial after adding subject prefix "hw/display/qxl-logger.c:".

Thanks,

/mjt

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/2] Check value for invalid negative values
  2015-06-11 13:17 ` [Qemu-devel] [PATCH 2/2] Check value for invalid negative values Frediano Ziglio
@ 2015-06-17 19:27   ` Michael Tokarev
  2015-06-18  9:58     ` Frediano Ziglio
  0 siblings, 1 reply; 8+ messages in thread
From: Michael Tokarev @ 2015-06-17 19:27 UTC (permalink / raw)
  To: Frediano Ziglio, Gerd Hoffmann; +Cc: qemu-trivial, qemu-devel

11.06.2015 16:17, Frediano Ziglio wrote:
> In qxl_v2n check that value is not negative.

Why do you think it is necessary?

Thanks,

/mjt

> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  hw/display/qxl-logger.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
> index d944d3f..faed869 100644
> --- a/hw/display/qxl-logger.c
> +++ b/hw/display/qxl-logger.c
> @@ -93,7 +93,7 @@ static const char *const spice_cursor_type[] = {
>  
>  static const char *qxl_v2n(const char *const n[], size_t l, int v)
>  {
> -    if (v >= l || !n[v]) {
> +    if (v < 0 || v >= l || !n[v]) {
>          return "???";
>      }
>      return n[v];

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/2] Check value for invalid negative values
  2015-06-17 19:27   ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
@ 2015-06-18  9:58     ` Frediano Ziglio
  2015-06-18 10:18       ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Frediano Ziglio @ 2015-06-18  9:58 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: qemu-trivial, Gerd Hoffmann, qemu-devel

For the same reason there is the v >= l test.
The v >= l test state that the value can be out of range so it not always a constant in the range.
Adding the v < 0 check for every invalid value. As these are executed only for logging should not be a performance penalty.
I also hope the compiler is able to optimize

if (v < 0 || v >= l)

with 

if ((unsigned) v >= l)

Frediano

> 
> 11.06.2015 16:17, Frediano Ziglio wrote:
> > In qxl_v2n check that value is not negative.
> 
> Why do you think it is necessary?
> 
> Thanks,
> 
> /mjt
> 
> > Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> > ---
> >  hw/display/qxl-logger.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
> > index d944d3f..faed869 100644
> > --- a/hw/display/qxl-logger.c
> > +++ b/hw/display/qxl-logger.c
> > @@ -93,7 +93,7 @@ static const char *const spice_cursor_type[] = {
> >  
> >  static const char *qxl_v2n(const char *const n[], size_t l, int v)
> >  {
> > -    if (v >= l || !n[v]) {
> > +    if (v < 0 || v >= l || !n[v]) {
> >          return "???";
> >      }
> >      return n[v];
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/2] Check value for invalid negative values
  2015-06-18  9:58     ` Frediano Ziglio
@ 2015-06-18 10:18       ` Gerd Hoffmann
  2015-06-18 10:45         ` Frediano Ziglio
  0 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2015-06-18 10:18 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: qemu-trivial, Michael Tokarev, qemu-devel

On Do, 2015-06-18 at 05:58 -0400, Frediano Ziglio wrote:
> For the same reason there is the v >= l test.
> The v >= l test state that the value can be out of range so it not always a constant in the range.
> Adding the v < 0 check for every invalid value. As these are executed only for logging should not be a performance penalty.
> I also hope the compiler is able to optimize
> 
> if (v < 0 || v >= l)
> 
> with 
> 
> if ((unsigned) v >= l)

Just make v explicitly unsigned?

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/2] Check value for invalid negative values
  2015-06-18 10:18       ` Gerd Hoffmann
@ 2015-06-18 10:45         ` Frediano Ziglio
  2015-06-18 13:56           ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Frediano Ziglio @ 2015-06-18 10:45 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-trivial, Michael Tokarev, qemu-devel

 
> On Do, 2015-06-18 at 05:58 -0400, Frediano Ziglio wrote:
> > For the same reason there is the v >= l test.
> > The v >= l test state that the value can be out of range so it not always a
> > constant in the range.
> > Adding the v < 0 check for every invalid value. As these are executed only
> > for logging should not be a performance penalty.
> > I also hope the compiler is able to optimize
> > 
> > if (v < 0 || v >= l)
> > 
> > with
> > 
> > if ((unsigned) v >= l)
> 
> Just make v explicitly unsigned?
> 
> cheers,
>   Gerd
> 

Do you mean in the prototype? Well, this could have side effect due to different conversions so is not a so trivial patch.
Explicitly casting to unsigned would do but is IMHO less easy to read that an explicit check.

Frediano

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/2] Check value for invalid negative values
  2015-06-18 10:45         ` Frediano Ziglio
@ 2015-06-18 13:56           ` Gerd Hoffmann
  0 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2015-06-18 13:56 UTC (permalink / raw)
  To: Frediano Ziglio; +Cc: qemu-trivial, Michael Tokarev, qemu-devel

On Do, 2015-06-18 at 06:45 -0400, Frediano Ziglio wrote:
>  > On Do, 2015-06-18 at 05:58 -0400, Frediano Ziglio wrote:
> > > For the same reason there is the v >= l test.
> > > The v >= l test state that the value can be out of range so it not always a
> > > constant in the range.
> > > Adding the v < 0 check for every invalid value. As these are executed only
> > > for logging should not be a performance penalty.
> > > I also hope the compiler is able to optimize
> > > 
> > > if (v < 0 || v >= l)
> > > 
> > > with
> > > 
> > > if ((unsigned) v >= l)
> > 
> > Just make v explicitly unsigned?
> > 
> > cheers,
> >   Gerd
> > 
> 
> Do you mean in the prototype?

Yes.

> Well, this could have side effect due to different conversions so is not a so trivial patch.

What side effects?  I don't expect any for in-range values, and how
exactly we catch out-of-range values doesn't really matter ...

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-06-18 13:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-11 13:17 [Qemu-devel] [PATCH 1/2] Constify some variable Frediano Ziglio
2015-06-11 13:17 ` [Qemu-devel] [PATCH 2/2] Check value for invalid negative values Frediano Ziglio
2015-06-17 19:27   ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2015-06-18  9:58     ` Frediano Ziglio
2015-06-18 10:18       ` Gerd Hoffmann
2015-06-18 10:45         ` Frediano Ziglio
2015-06-18 13:56           ` Gerd Hoffmann
2015-06-17 19:23 ` [Qemu-devel] [PATCH 1/2] Constify some variable Michael Tokarev

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.