[Cbe-oss-dev] [PATCH] Fixes for libspe-1.2.0 build-time warnings, inlined

Julio M. Merino Vidal jmerino at ac.upc.edu
Wed Mar 21 21:09:58 EST 2007


Hi,

I've been asked privately to resend all my patches for libspe-1.2.0, but
inlining them for easier review.  Here they go.  Each patch has a header
that explains what it does.

To refresh your memory, here is part of my original mail:

Here go some patches to fix several warnings when building libspe (both
for the -m32 and -m64 cases).  These are, specially, to resolve issues
when -DDEBUG is given.  They also fix a problem with the DEBUG_PRINTF
macro that arose in custom code; more details are given in the logs.

I'm wondering if these changes could be incorporated upstream.

Kind regards,

----- fix-build-warnings.diff

Subject: Fix a couple of build-time warnings

This patch fixes a couple of build-time warnings that appear when building
the code without any specific options.
Index: libspe-1.2.0/spethreads.c
===================================================================
--- libspe-1.2.0.orig/spethreads.c
+++ libspe-1.2.0/spethreads.c
@@ -405,7 +405,7 @@ int spe_count_physical_spes(void)
 		perror("dirlist");
 		return -1;
 	}
-	while(dptr=readdir(dirp))
+	while((dptr=readdir(dirp)) != NULL)
 	{
 		ret++;
 	}
@@ -487,7 +487,6 @@ int
 spe_recycle(speid_t spe, spe_program_handle_t *handle)
 {
 	int ret;
-	char buf = 0;
 	struct thread_store *thread = (struct thread_store *)spe;
 
 	if (!(thread->flags & SPE_ISOLATE)) {

----- fix-debug-build-warnings.diff

Subject: Fix build-time warnings when DEBUG is defined

This patch files multiple built-time warnings that arise when the DEBUG build
option is set.  These basically resolve inconsistences in printf(3) formatting
flags and their corresponding parameters.
Index: libspe-1.2.0/default_c99_handler.c
===================================================================
--- libspe-1.2.0.orig/default_c99_handler.c
+++ libspe-1.2.0/default_c99_handler.c
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <stdio.h>
+#include <stdint.h>
 #include <string.h>
 #include <ctype.h>
 #include <errno.h>
@@ -152,7 +153,7 @@ typedef unsigned long long __va_elem;
 
 #define CHECK_C99_OPCODE(_op)                               		\
     if (SPE_C99_OP(opdata) != SPE_C99_ ## _op) { 			\
-	DEBUG_PRINTF("OPCODE (0x%x) mismatch.\n", SPE_C99_OP(opdata)); 	\
+	DEBUG_PRINTF("OPCODE (0x%lx) mismatch.\n", SPE_C99_OP(opdata));	\
         return 1;                                       		\
     }
 
@@ -897,7 +898,7 @@ int default_c99_handler_tmpnam(char *ls,
 
     tmpnam(s);
 
-    PUT_LS_RC(s, 0, 0, 0);
+    PUT_LS_RC((uint32_t)((intptr_t)s & 0xffffffff), 0, 0, 0);
     return 0;
 }
 
Index: libspe-1.2.0/default_posix1_handler.c
===================================================================
--- libspe-1.2.0.orig/default_posix1_handler.c
+++ libspe-1.2.0/default_posix1_handler.c
@@ -123,7 +123,7 @@ enum {
 
 #define CHECK_POSIX1_OPCODE(_op)                                          \
     if (SPE_POSIX1_OP(opdata) != SPE_POSIX1_ ## _op) {                    \
-        DEBUG_PRINTF("OPCODE (0x%x) mismatch.\n", SPE_POSIX1_OP(opdata)); \
+        DEBUG_PRINTF("OPCODE (0x%lx) mismatch.\n", SPE_POSIX1_OP(opdata)); \
         return 1;                                                         \
     }
 
Index: libspe-1.2.0/dma.c
===================================================================
--- libspe-1.2.0.orig/dma.c
+++ libspe-1.2.0/dma.c
@@ -69,8 +69,8 @@ static int spe_do_mfc_put(speid_t spe, u
 	};
 	int ret;
 
-	DEBUG_PRINTF("queuing DMA %x %lx %x %x %x %x\n", parm.lsa,
-		parm.ea, parm.size, parm.tag, parm.class, parm.cmd);
+	DEBUG_PRINTF("queuing DMA %x %jx %x %x %x %x\n", parm.lsa,
+		(uintmax_t)parm.ea, parm.size, parm.tag, parm.class, parm.cmd);
 
 	if (!srch_thread(spe))
 		return -1;
@@ -103,8 +103,8 @@ static int spe_do_mfc_get(speid_t spe, u
 	};
 	int ret;
 
-	DEBUG_PRINTF("queuing DMA %x %lx %x %x %x %x\n", parm.lsa,
-		parm.ea, parm.size, parm.tag, parm.class, parm.cmd);
+	DEBUG_PRINTF("queuing DMA %x %jx %x %x %x %x\n", parm.lsa,
+		(uintmax_t)parm.ea, parm.size, parm.tag, parm.class, parm.cmd);
 
 	if (!srch_thread(spe))
 		return -1;
Index: libspe-1.2.0/libspe.h
===================================================================
--- libspe-1.2.0.orig/libspe.h
+++ libspe-1.2.0/libspe.h
@@ -31,6 +31,8 @@ extern "C"
 
 typedef void *speid_t;
 typedef void *spe_gid_t;
+#define SPE_PRINT_FMT_SPEID_T	"%p"
+#define SPE_PRINT_FMT_SPE_GID_T	"%p"
 
 /* Flags for spe_create_thread
  */
Index: libspe-1.2.0/spe.c
===================================================================
--- libspe-1.2.0.orig/spe.c
+++ libspe-1.2.0/spe.c
@@ -234,7 +234,7 @@ static int spe_setup_mem(struct thread_s
 	}
 
 	/* Add SPE exec program */
-	DEBUG_PRINTF ("Add exec prog dst:0x%04x size:0x%04x\n",
+	DEBUG_PRINTF ("Add exec prog dst:0x%04x size:%04zx\n",
 		      SPE_LDR_START, sizeof (spe_ldr));
 	memcpy (spe_ld_buf + SPE_LDR_START, &spe_ldr, sizeof (spe_ldr));
 
@@ -278,7 +278,7 @@ static int spe_setup_mem(struct thread_s
 	spe_params.gpr6[2] = 0;
 	spe_params.gpr6[3] = 0;
 
-	DEBUG_PRINTF ("Add exec param dst:0x%04x size:0x%04x\n",
+	DEBUG_PRINTF ("Add exec param dst:0x%04x size:%04zx\n",
 		      SPE_PARAM_START, sizeof (spe_params));
 	memcpy (spe_ld_buf + SPE_PARAM_START, &spe_params,
 		sizeof (spe_params));
Index: libspe-1.2.0/spethreads.c
===================================================================
--- libspe-1.2.0.orig/spethreads.c
+++ libspe-1.2.0/spethreads.c
@@ -383,7 +383,8 @@ int spe_group_max (spe_gid_t spe_gid)
 		return -1;
 	}
 	
-	DEBUG_PRINTF ("spu_group_max(0x%x)\n", spe_gid);
+	DEBUG_PRINTF ("spu_group_max(0x" SPE_PRINT_FMT_SPE_GID_T ")\n",
+                      spe_gid);
 
 	return MAX_THREADS_PER_GROUP;	
 }
@@ -442,7 +443,7 @@ spe_create_thread (spe_gid_t gid, spe_pr
 		return NULL;
 	}
 	
-	DEBUG_PRINTF ("spe_create_thread(%p, %p, %p, %p, 0x%x, 0x%x)\n",
+	DEBUG_PRINTF ("spe_create_thread(%p, %p, %p, %p, 0x%lx, 0x%x)\n",
 		      gid, handle, argp, envp, mask, flags);
 
 	if (check_env)
@@ -540,7 +541,8 @@ spe_wait(speid_t speid, int *status, int
 		return -1;
 	}	
 	
-	DEBUG_PRINTF("spu_wait(0x%x, %p, 0x%x)\n", speid, status, options);
+	DEBUG_PRINTF("spu_wait(0x" SPE_PRINT_FMT_SPEID_T ", %p, 0x%x)\n",
+                     speid, status, options);
 /*
 	if (options & WNOHANG)
 	{
----- make-debug-printf-work-in-single-instruction-bodies.diff

Subject: Make DEBUG_PRINTF work in single-instruction bodies

The original DEBUG_PRINTF failed to work in code such as the following:

    if (...)
        DEBUG_PRINTF("A message");
    else
        ... do something else ...

This is because the trailing semicolon after DEBUG_PRINTF was being
interpreted after the closing bracked of the macro's contents.  The
fix makes the macro use a do/while(0) construction so that any trailing
semicolon is harmless.  In fact, it must now be present for the macro
to work properly.
Index: libspe-1.2.0/default_syscall_handler.c
===================================================================
--- libspe-1.2.0.orig/default_syscall_handler.c
+++ libspe-1.2.0/default_syscall_handler.c
@@ -28,7 +28,7 @@
 
 #include <default_syscall_handler.h>
 
-#define __PRINTF(fmt, args...) { fprintf(stderr,fmt , ## args); }
+#define __PRINTF(fmt, args...) do { fprintf(stderr,fmt , ## args); } while (0)
 #ifdef DEBUG
 #define DEBUG_PRINTF(fmt, args...) __PRINTF(fmt , ## args)
 #else
Index: libspe-1.2.0/dma.c
===================================================================
--- libspe-1.2.0.orig/dma.c
+++ libspe-1.2.0/dma.c
@@ -27,7 +27,7 @@
 
 #include "spe.h"
 
-#define __PRINTF(fmt, args...) { fprintf(stderr,fmt , ## args); }
+#define __PRINTF(fmt, args...) do { fprintf(stderr,fmt , ## args); } while (0)
 #ifdef DEBUG
 #define DEBUG_PRINTF(fmt, args...) __PRINTF(fmt , ## args)
 #else
Index: libspe-1.2.0/elf_loader.c
===================================================================
--- libspe-1.2.0.orig/elf_loader.c
+++ libspe-1.2.0/elf_loader.c
@@ -33,7 +33,7 @@ extern int __env_spu_debug_start;
 extern int __env_spu_info;
 static void display_debug_output(Elf32_Ehdr *elf_start, Elf32_Shdr *sh);
 
-#define __PRINTF(fmt, args...) { fprintf(stderr,fmt , ## args); }
+#define __PRINTF(fmt, args...) do { fprintf(stderr,fmt , ## args); } while (0)
 #ifdef DEBUG
 #define DEBUG_PRINTF(fmt, args...) __PRINTF(fmt , ## args)
 #define TAG DEBUG_PRINTF("TAG: %d@%s\n", __LINE__, __FILE__);
Index: libspe-1.2.0/handler_utils.h
===================================================================
--- libspe-1.2.0.orig/handler_utils.h
+++ libspe-1.2.0/handler_utils.h
@@ -29,7 +29,7 @@ struct spe_reg128 {
 #define LS_ADDR_MASK                  (LS_SIZE - 1)
 #endif /* LS_SIZE */
 
-#define __PRINTF(fmt, args...) { fprintf(stderr,fmt , ## args); }
+#define __PRINTF(fmt, args...) do { fprintf(stderr,fmt , ## args); } while (0)
 #ifdef DEBUG
 #define DEBUG_PRINTF(fmt, args...) __PRINTF(fmt , ## args)
 #else
Index: libspe-1.2.0/ps.c
===================================================================
--- libspe-1.2.0.orig/ps.c
+++ libspe-1.2.0/ps.c
@@ -30,7 +30,7 @@
 
 #include "spe.h"
 
-#define __PRINTF(fmt, args...) { fprintf(stderr,fmt , ## args); }
+#define __PRINTF(fmt, args...) do { fprintf(stderr,fmt , ## args); } while (0)
 #ifdef DEBUG
 #define DEBUG_PRINTF(fmt, args...) __PRINTF(fmt , ## args)
 #else
Index: libspe-1.2.0/signal.c
===================================================================
--- libspe-1.2.0.orig/signal.c
+++ libspe-1.2.0/signal.c
@@ -30,7 +30,7 @@
 
 #include "spe.h"
 
-#define __PRINTF(fmt, args...) { fprintf(stderr,fmt , ## args); }
+#define __PRINTF(fmt, args...) do { fprintf(stderr,fmt , ## args); } while (0)
 #ifdef DEBUG
 #define DEBUG_PRINTF(fmt, args...) __PRINTF(fmt , ## args)
 #else
Index: libspe-1.2.0/spe.h
===================================================================
--- libspe-1.2.0.orig/spe.h
+++ libspe-1.2.0/spe.h
@@ -21,7 +21,7 @@
 
 /* Definitions
  */
-#define __PRINTF(fmt, args...) { fprintf(stderr,fmt , ## args); }
+#define __PRINTF(fmt, args...) do { fprintf(stderr,fmt , ## args); } while (0)
 #ifdef DEBUG
 #define DEBUG_PRINTF(fmt, args...) __PRINTF(fmt , ## args)
 #else



More information about the cbe-oss-dev mailing list