cracklib-devel

[Cracklib-devel] dealing with newer zlib and build time warnings

[Mike F. <va...@ge...>]

the cracklib packer does things like:
#ifdef HAVE_ZLIB_H
		if (pdesc.flags & PFOR_USEZLIB)
			gzclose(pdesc.dfp);
		else
#endif
			fclose(pdesc.dfp);

where dfp and such are FILE* pointers.  with newer zlib, we see warnings like:

packlib.c: In function 'PWOpen':
packlib.c:128:4: warning: passing argument 1 of 'gzclose' from incompatible 
pointer type [enabled by default]
In file included from packlib.c:11:0:
/usr/include/zlib.h:1511:12: note: expected 'gzFile' but argument is of type 
'struct FILE *'
packlib.c:165:4: warning: passing argument 1 of 'gzclose' from incompatible 
pointer type [enabled by default]

this is because gzopen/gzclose use gzFile rather than FILE*

one way to fix this would be to add (void*) casts to all zlib related funcs, 
but that sucks.  we could change these to unions like so:
	typedef struct
	{
		union {
			FILE *fp;
			gzFile *gfp;
		} dfp;
but that sucks because we'd have to update all the call points to use the 
right union member.  another idea (which also sucks, but maybe less so) is to 
change the pointer type to void* (see below).

thoughts ?  all three of these ideas should not impact the ABI.

--- lib/packer.h	(revision 219)
+++ lib/packer.h	(working copy)
@@ -52,9 +52,9 @@ struct pi_header
 
 typedef struct
 {
-    FILE *ifp;
-    FILE *dfp;
-    FILE *wfp;
+    void *ifp;
+    void *dfp;
+    void *wfp;
 
     uint32_t flags;
 #define PFOR_WRITE	0x0001
--- lib/packlib.c	(revision 219)
+++ lib/packlib.c	(working copy)
@@ -34,9 +34,9 @@ struct pi_header64
 
 typedef struct
 {
-    FILE *ifp;
-    FILE *dfp;
-    FILE *wfp;
+    void *ifp;
+    void *dfp;
+    void *wfp;
     uint64_t flags;
     uint64_t hwms[256];
     struct pi_header64 header;
@@ -72,9 +72,9 @@ PWOpen(prefix, mode)
     char iname[STRINGSIZE];
     char dname[STRINGSIZE];
     char wname[STRINGSIZE];
-    FILE *dfp;
-    FILE *ifp;
-    FILE *wfp;
+    void *dfp;
+    void *ifp;
+    void *wfp;
 
     if (pdesc.header.pih_magic == PIH_MAGIC)
     {
-mike

Re: [Cracklib-devel] dealing with newer zlib and build time warnings

[Jan D. <ja...@di...>]

On Sat, Jun 01, 2013 at 10:02:40PM -0400, Mike Frysinger wrote:
> the cracklib packer does things like:
> #ifdef HAVE_ZLIB_H
> 		if (pdesc.flags & PFOR_USEZLIB)
> 			gzclose(pdesc.dfp);
> 		else
> #endif
> 			fclose(pdesc.dfp);
> 
> where dfp and such are FILE* pointers.  with newer zlib, we see warnings like:
> 
> packlib.c: In function 'PWOpen':
> packlib.c:128:4: warning: passing argument 1 of 'gzclose' from incompatible 
> pointer type [enabled by default]
> In file included from packlib.c:11:0:
> /usr/include/zlib.h:1511:12: note: expected 'gzFile' but argument is of type 
> 'struct FILE *'
> packlib.c:165:4: warning: passing argument 1 of 'gzclose' from incompatible 
> pointer type [enabled by default]
> 
> this is because gzopen/gzclose use gzFile rather than FILE*
> 
> one way to fix this would be to add (void*) casts to all zlib related funcs, 
> but that sucks.  we could change these to unions like so:
> 	typedef struct
> 	{
> 		union {
> 			FILE *fp;
> 			gzFile *gfp;
> 		} dfp;
> but that sucks because we'd have to update all the call points to use the 
> right union member.  another idea (which also sucks, but maybe less so) is to 
> change the pointer type to void* (see below).
> 
> thoughts ?  all three of these ideas should not impact the ABI.

I aggree with you that the third variant proposed (using a void* pointer)
looks as it sucks the least. The second variant would more clearly express
the intent of the pointer but I am not sure whether it is worth the trouble
to change all FILE* pointers to the union type (I counted 90 appearances of
FILE pointer uses throughout the cracklib2 codebase).


Best regards
Jan

-- 
Jan Dittberner - Debian Developer
GPG-key: 4096R/558FB8DD 2009-05-10
         B2FF 1D95 CE8F 7A22 DF4C  F09B A73E 0055 558F B8DD
http://www.dittberner.info/

Re: [Cracklib-devel] dealing with newer zlib and build time warnings

[Mike F. <va...@ge...>]

On Saturday 01 June 2013 22:02:40 Mike Frysinger wrote:
> right union member.  another idea (which also sucks, but maybe less so) is
> to change the pointer type to void* (see below).

i committed this
-mike