Re: [Spice-devel] [PATCH] tests: fix compilation with -Wall -Werror

2011-03-22 Thread Christophe Fergeau
On Mon, Mar 21, 2011 at 06:47:19PM +0100, Christophe Fergeau wrote:
  On Mon, Mar 21, 2011 at 05:24:54PM +0200, Alon Levy wrote:
  
  btw, while you're at it, mind trying to compile with clang?
 
 I ran some tests, spice compiled fine with clang using make CC=clang
 CXX=clang++ CFLAGS=-Wall -Werror -g3 -ggdb3 -O3 CXXFLAGS=-Wall -Werror
 -g3 -ggdb3 -O3, I don't know if there is a magical clang warning flags
 to set in addition to that.
 I hit a compilation error with spice-gtk which I'm trying to figure out.

The compilation error was due to a warning coming from the use of
G_DEFINE_BOXED_TYPE, I reported the issue here
https://bugzilla.gnome.org/show_bug.cgi?id=449565#c53
After hacking around it, the compilation works fine.

Christophe


pgpwg2IYy5gVn.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH] tests: fix compilation with -Wall -Werror

2011-03-21 Thread Alon Levy
On Mon, Mar 21, 2011 at 04:20:29PM +0100, Christophe Fergeau wrote:
 When compiling spice with make CFLAGS=-g3 -ggdb3 -O0 -Wall -Werror,
 the build broken because of a few unused variables/missing returns.
 This patch fixes these warnings.

Just one comment inline. Looks good otherwise.

 ---
  server/tests/basic_event_loop.c   |2 +-
  server/tests/test_display_base.c  |   16 +---
  server/tests/test_empty_success.c |3 +++
  3 files changed, 5 insertions(+), 16 deletions(-)
 
 diff --git a/server/tests/basic_event_loop.c b/server/tests/basic_event_loop.c
 index 7e8a1df..2fe1b69 100644
 --- a/server/tests/basic_event_loop.c
 +++ b/server/tests/basic_event_loop.c
 @@ -319,7 +319,7 @@ void basic_event_loop_mainloop(void)
  if ((next_timer = get_next_timer()) != NULL) {
  calc_next_timeout(next_timer, next_timer_timeout);
  timeout = next_timer_timeout;
 -DPRINTF(1, timeout of %d.%06d,
 +DPRINTF(1, timeout of %zd.%06zd,
  timeout-tv_sec, timeout-tv_usec);
  } else {
  timeout = NULL;
 diff --git a/server/tests/test_display_base.c 
 b/server/tests/test_display_base.c
 index c87d2c7..9cb7ad0 100644
 --- a/server/tests/test_display_base.c
 +++ b/server/tests/test_display_base.c
 @@ -40,7 +40,6 @@ static void test_spice_destroy_update(SimpleSpiceUpdate 
 *update)
  
  #define SINGLE_PART 8
  static const int angle_parts = 64 / SINGLE_PART;
 -static int angle = 0;
  static int unique = 1;
  static int color = -1;
  static int c_i = 0;
 @@ -354,7 +353,6 @@ static int num_simple_commands = 0;
  static void produce_command()
  {
  static int target_surface = 0;
 -static int simple_command_index = 0;
  static int cmd_index = 0;
  
  ASSERT(num_simple_commands);
 @@ -449,19 +447,6 @@ static struct {
  uint8_t data[CURSOR_WIDTH * CURSOR_HEIGHT * 4]; // 32bit per pixel
  } cursor;
  
 -static void init_cursor()
 -{
 -cursor.cursor.header.unique = 0; // TODO ??
 -cursor.cursor.header.type = SPICE_CURSOR_TYPE_COLOR32;
 -cursor.cursor.header.width = CURSOR_WIDTH;
 -cursor.cursor.header.height = CURSOR_HEIGHT;
 -cursor.cursor.header.hot_spot_x = 0;
 -cursor.cursor.header.hot_spot_y = 0;
 -cursor.cursor.data_size = CURSOR_WIDTH * CURSOR_HEIGHT;
 -cursor.cursor.chunk.data_size = cursor.cursor.data_size;
 -cursor.cursor.chunk.prev_chunk = cursor.cursor.chunk.next_chunk = 0;
 -}
 -

Well, this is test code, I would want to get back to this eventually, so
could you turn this into a non static (and then the compiler won't complain)
instead? if that doesn't work even #if 0 is better. (again, test code).

  static int get_cursor_command(QXLInstance *qin, struct QXLCommandExt *ext)
  {
  static int color = 0;
 @@ -515,6 +500,7 @@ static void notify_update(QXLInstance *qin, uint32_t 
 update_id)
  static int flush_resources(QXLInstance *qin)
  {
  printf(%s\n, __func__);
 +return TRUE;
  }
  
  QXLInterface display_sif = {
 diff --git a/server/tests/test_empty_success.c 
 b/server/tests/test_empty_success.c
 index e747e40..97aa772 100644
 --- a/server/tests/test_empty_success.c
 +++ b/server/tests/test_empty_success.c
 @@ -1,8 +1,10 @@
 +#include stdlib.h
  #include strings.h
  #include spice.h
  
  SpiceTimer* timer_add(SpiceTimerFunc func, void *opaque)
  {
 +return NULL;
  }
  
  void timer_start(SpiceTimer *timer, uint32_t ms)
 @@ -19,6 +21,7 @@ void timer_remove(SpiceTimer *timer)
  
  SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void 
 *opaque)
  {
 +return NULL;
  }
  
  void watch_update_mask(SpiceWatch *watch, int event_mask)
 -- 
 1.7.4
 
 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH] tests: fix compilation with -Wall -Werror

2011-03-21 Thread Christophe Fergeau
On Mon, Mar 21, 2011 at 05:24:54PM +0200, Alon Levy wrote:
 
 Well, this is test code, I would want to get back to this eventually, so
 could you turn this into a non static (and then the compiler won't complain)
 instead? if that doesn't work even #if 0 is better. (again, test code).
 

Adding 'static void init_cursor() __attribute__((unused));' before the
function definition avoid the warning, but I don't know how (in)dependant
from GCC the code base should be? If that's too specific, I'll go with the
#if 0. I'm not a big fan of making this function non-static in case someone
uses this code as a base for something that is not a test and forget to
readd the missing 'static'

Thanks for the review,

Christophe


pgpbzkSNqqGBe.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH] tests: fix compilation with -Wall -Werror

2011-03-21 Thread Christophe Fergeau
On Mon, Mar 21, 2011 at 05:32:20PM +0200, Alon Levy wrote:
 On Mon, Mar 21, 2011 at 05:24:54PM +0200, Alon Levy wrote:
  On Mon, Mar 21, 2011 at 04:20:29PM +0100, Christophe Fergeau wrote:
   When compiling spice with make CFLAGS=-g3 -ggdb3 -O0 -Wall -Werror,
   the build broken because of a few unused variables/missing returns.
   This patch fixes these warnings.
  
 
 btw, while you're at it, mind trying to compile with clang?

I ran some tests, spice compiled fine with clang using make CC=clang
CXX=clang++ CFLAGS=-Wall -Werror -g3 -ggdb3 -O3 CXXFLAGS=-Wall -Werror
-g3 -ggdb3 -O3, I don't know if there is a magical clang warning flags
to set in addition to that.
I hit a compilation error with spice-gtk which I'm trying to figure out.

Christophe


pgpBiEQ3y9Fa3.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel