PATCH: silencing compiler warnings from gcc 4

PATCH: silencing compiler warnings from gcc 4

[Wayne Davison]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

Gcc 4 outputs a bunch of compiler warnings about string pointers that
differ in signedness.  The warnings in a zsh build are limited to the
zle string functions defined in zle.h.  I came up with 2 solutions:
(1) change the ZS_str* macros to cast their args to (char*), or (2)
introduce some static inline functions to handle the casts.  The
advantage of the latter is that it doesn't hide pointer-conversion
errors behind forced casts.  For instance, after I compiled the inline
version it showed several places in the code that were using ZWC() on
strings instead of ZWS() (which I already fixed and checked in).

As for the portability of "inline", there is an autoconf macro that I
included, AC_C_INLINE, that redefines the word "inline" as appropriate.
The rsync package used a static inline function in its rsync.h for many
years (thought I eliminated it recently), so it would seem to be quite
portable.

Comments?

..wayne..

[-- Attachment #2: signedness-inline.diff --]
[-- Type: text/plain, Size: 1556 bytes --]

--- configure.ac	1 Aug 2005 09:54:56 -0000	1.37
+++ configure.ac	11 Aug 2005 18:42:00 -0000
@@ -437,6 +437,7 @@ AC_SUBST(LIBLDFLAGS)dnl
 AC_PROG_CPP                 dnl Figure out how to run C preprocessor.
 AC_PROG_GCC_TRADITIONAL     dnl Do we need -traditional flag for gcc.
 AC_C_CONST                  dnl Does compiler support `const'.
+AC_C_INLINE
 
 dnl Default preprocessing on Mac OS X produces warnings
 case "$host_os" in
--- Src/Zle/zle.h	10 Aug 2005 10:56:41 -0000	1.15
+++ Src/Zle/zle.h	11 Aug 2005 18:42:00 -0000
@@ -89,10 +89,20 @@ typedef int ZLE_INT_T;
 #define ZS_memmove memmove
 #define ZS_memset memset
 #define ZS_memcmp memcmp
-#define ZS_strlen strlen
-#define ZS_strcpy strcpy
-#define ZS_strncpy strncpy
-#define ZS_strncmp strncmp
+
+static inline size_t ZS_strlen(ZLE_STRING_T s);
+static inline ZLE_STRING_T ZS_strcpy(ZLE_STRING_T t, ZLE_STRING_T f);
+static inline ZLE_STRING_T ZS_strncpy(ZLE_STRING_T t, ZLE_STRING_T f, size_t l);
+static inline int ZS_strncmp(ZLE_STRING_T s1, ZLE_STRING_T s2, size_t l);
+
+static inline size_t ZS_strlen(ZLE_STRING_T s)
+{ return strlen((char*)s); }
+static inline ZLE_STRING_T ZS_strcpy(ZLE_STRING_T t, ZLE_STRING_T f)
+{ return (ZLE_STRING_T)strcpy((char*)t, (char*)f); }
+static inline ZLE_STRING_T ZS_strncpy(ZLE_STRING_T t, ZLE_STRING_T f, size_t l)
+{ return (ZLE_STRING_T)strncpy((char*)t, (char*)f, l); }
+static inline int ZS_strncmp(ZLE_STRING_T s1, ZLE_STRING_T s2, size_t l)
+{ return strncmp((char*)s1, (char*)s2, l); }
 
 #define ZC_iblank iblank
 #define ZC_icntrl icntrl

[-- Attachment #3: signedness.diff --]
[-- Type: text/plain, Size: 603 bytes --]

--- Src/Zle/zle.h	10 Aug 2005 10:56:41 -0000	1.15
+++ Src/Zle/zle.h	11 Aug 2005 18:20:16 -0000
@@ -89,10 +89,10 @@ typedef int ZLE_INT_T;
 #define ZS_memmove memmove
 #define ZS_memset memset
 #define ZS_memcmp memcmp
-#define ZS_strlen strlen
-#define ZS_strcpy strcpy
-#define ZS_strncpy strncpy
-#define ZS_strncmp strncmp
+#define ZS_strlen(s) strlen((char*)(s))
+#define ZS_strcpy(t,f) strcpy((char*)(t),(char*)(f))
+#define ZS_strncpy(t,f,l) strncpy((char*)(t),(char*)(f),(l))
+#define ZS_strncmp(s1,s2,l) strncmp((char*)(s1),(char*)(s2),(l))
 
 #define ZC_iblank iblank
 #define ZC_icntrl icntrl

Re: PATCH: silencing compiler warnings from gcc 4

[Peter Stephenson]

Wayne Davison <wayned@users.sourceforge.net> wrote:
> Gcc 4 outputs a bunch of compiler warnings about string pointers that
> differ in signedness.  The warnings in a zsh build are limited to the
> zle string functions defined in zle.h.  I came up with 2 solutions:
> (1) change the ZS_str* macros to cast their args to (char*), or (2)
> introduce some static inline functions to handle the casts.  The
> advantage of the latter is that it doesn't hide pointer-conversion
> errors behind forced casts.  For instance, after I compiled the inline
> version it showed several places in the code that were using ZWC() on
> strings instead of ZWS() (which I already fixed and checked in).

I was wondering about these.  I'm a bit loath either to cast away any
typesafety quite so blithely as the first change would do, but I'm not that
keen on putting too much work into non-standard features (which will
reduce to the first case where inline isn't available).  I believe even C99
doesn't allow every use of inline that gcc does.  However, given that we're
stuck with casts of some sort, and we do a lot of the development with gcc,
possibly the second option is the best way to go.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************

Re: PATCH: silencing compiler warnings from gcc 4

[Peter Stephenson]

Wayne Davison <wayned@users.sourceforge.net> wrote:
> As for the portability of "inline", there is an autoconf macro that I
> included, AC_C_INLINE, that redefines the word "inline" as appropriate.
> The rsync package used a static inline function in its rsync.h for many
> years (thought I eliminated it recently), so it would seem to be quite
> portable.

Hmm... did I read this the right way?  I had assumed that if inline wasn't
available we'd use a macro.  But maybe it simply means if inline isn't
available, we end up calling two functions to get a simple strlen().  If
that's the case it seems to be over the top... I think we would need
something like


#ifdef <inline is available>

static inline size_t ZS_strlen(ZLE_STRING_T s)
{ return strlen((char*)s); }

#else

#define ZS_strlen(s) strlen((char *)s)

#endif


Then we still get debugging information from gcc without unnecessarily
increasing the overhead with other compilers.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************

Re: PATCH: silencing compiler warnings from gcc 4

[Wayne Davison]

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

On Fri, Aug 12, 2005 at 12:31:14PM +0100, Peter Stephenson wrote:
> But maybe it simply means if inline isn't available, we end up calling
> two functions to get a simple strlen().

Yes, that's what my patch did.  I had been thinking about optimizing
that in the same way you suggested.  I'm currently debating whether it
might be enough to only use the inline functions if __GNUC__ is defined
(since the value of the inline functions is at compile-time, not
runtime).  The only argument against this that I can come up with is
that there may be some conditional code (at some point) that won't get
compiled by gcc, and thus it won't get type-checked.  The argument for
only doing this with gcc is that we're sure that the inline functions
are doing the right thing.

Since it seems that we agree that this is a good thing for at least gcc,
I've checked in a patch that uses inline functions for gcc and attached
that patch to this email.  If we want to extend this to use inline
functions on every compiler that supports inline functions, the second
attached patch should accomplish that -- it changes configure to add a
new define, INLINE_AVAILABLE, if inline functions are possible.

..wayne..

[-- Attachment #2: signedness.diff --]
[-- Type: text/plain, Size: 1098 bytes --]

--- Src/Zle/zle.h	10 Aug 2005 10:56:41 -0000	1.15
+++ Src/Zle/zle.h	12 Aug 2005 13:12:37 -0000
@@ -89,10 +89,22 @@ typedef int ZLE_INT_T;
 #define ZS_memmove memmove
 #define ZS_memset memset
 #define ZS_memcmp memcmp
-#define ZS_strlen strlen
-#define ZS_strcpy strcpy
-#define ZS_strncpy strncpy
-#define ZS_strncmp strncmp
+
+#ifdef __GNUC__
+static inline size_t ZS_strlen(ZLE_STRING_T s)
+{ return strlen((char*)s); }
+static inline ZLE_STRING_T ZS_strcpy(ZLE_STRING_T t, ZLE_STRING_T f)
+{ return (ZLE_STRING_T)strcpy((char*)t, (char*)f); }
+static inline ZLE_STRING_T ZS_strncpy(ZLE_STRING_T t, ZLE_STRING_T f, size_t l)
+{ return (ZLE_STRING_T)strncpy((char*)t, (char*)f, l); }
+static inline int ZS_strncmp(ZLE_STRING_T s1, ZLE_STRING_T s2, size_t l)
+{ return strncmp((char*)s1, (char*)s2, l); }
+#else
+#define ZS_strlen(s) strlen((char*)(s))
+#define ZS_strcpy(t,f) strcpy((char*)(t),(char*)(f))
+#define ZS_strncpy(t,f,l) strncpy((char*)(t),(char*)(f),(l))
+#define ZS_strncmp(s1,s2,l) strncmp((char*)(s1),(char*)(s2),(l))
+#endif
 
 #define ZC_iblank iblank
 #define ZC_icntrl icntrl

[-- Attachment #3: signedness2.patch --]
[-- Type: text/plain, Size: 978 bytes --]

--- configure.ac	1 Aug 2005 09:54:56 -0000	1.37
+++ configure.ac	12 Aug 2005 13:14:41 -0000
@@ -437,6 +437,14 @@ AC_SUBST(LIBLDFLAGS)dnl
 AC_PROG_CPP                 dnl Figure out how to run C preprocessor.
 AC_PROG_GCC_TRADITIONAL     dnl Do we need -traditional flag for gcc.
 AC_C_CONST                  dnl Does compiler support `const'.
+AC_C_INLINE
+
+AH_TEMPLATE([INLINE_AVAILABLE],
+[Define to 1 if inline functions are possible.])
+case $ac_cv_c_inline in
+  no) ;;
+  *) AC_DEFINE(INLINE_AVAILABLE) ;;
+esac
 
 dnl Default preprocessing on Mac OS X produces warnings
 case "$host_os" in
--- Src/Zle/zle.h	12 Aug 2005 13:12:37 -0000	1.16
+++ Src/Zle/zle.h	12 Aug 2005 13:14:41 -0000
@@ -90,7 +90,7 @@ typedef int ZLE_INT_T;
 #define ZS_memset memset
 #define ZS_memcmp memcmp
 
-#ifdef __GNUC__
+#ifdef INLINE_AVAILABLE
 static inline size_t ZS_strlen(ZLE_STRING_T s)
 { return strlen((char*)s); }
 static inline ZLE_STRING_T ZS_strcpy(ZLE_STRING_T t, ZLE_STRING_T f)