Re: libc warnings

[Philip Guenther]

On Thu, Apr 4, 2013 at 6:49 PM, Ted Unangst t...@tedunangst.com wrote:
 To prevent the future recurrence of some rather serious libc build
 bugs, such as a macro like strong_alias suddenly turning into an
 implicit function prototype, I think we should add some warnings and
 use as much -Werror as possible. (I'm not entirely sure this would
 have caught the missing strong_alias macro, but it seems like a good
 idea anyway.)

 This adds a (very) few warnings and fixes some of the fallout. The
 diff is a bit of a mix of things, since I initially tried -Wall before
 I ran away screaming. Over time, maybe some brave pioneers can expand
 the frontier.
...
 +CFLAGS+=-Wimplicit -Wparentheses -Wbounded -Werror

The make release process builds parts of libc with extra #defines to
disable stuff; have you done that with these flags to verify there
aren't warnings in those chunks?


 --- regex/regcomp.c 7 Nov 2011 09:58:27 -   1.21
 +++ regex/regcomp.c 5 Apr 2013 01:22:39 -
 @@ -171,8 +171,7 @@ regcomp(regex_t *preg, const char *patte
 len = strlen((char *)pattern);

 /* do the mallocs early so failure handling is easy */
 -   g = (struct re_guts *)malloc(sizeof(struct re_guts) +
 -   (NC-1)*sizeof(cat_t));
 +   g = (struct re_guts *)malloc(sizeof(struct re_guts));
...
 --- regex/regex2.h  30 Nov 2004 17:04:23 -  1.7
 +++ regex/regex2.h  5 Apr 2013 01:22:39 -
 @@ -149,7 +149,7 @@ struct re_guts {
 int backrefs;   /* does it use back references? */
 sopno nplus;/* how deep does it nest +s? */
 /* catspace must be last */
 -   cat_t catspace[1];  /* actually [NC] */
 +   cat_t catspace[NC]; /* actually [NC] */
  };

Please tell me that used to actually _use_ the struct hack and didn't
always allocate the full size!



 --- regex/regexec.c 5 Aug 2005 13:03:00 -   1.11
 +++ regex/regexec.c 5 Apr 2013 01:22:39 -
 @@ -140,6 +140,8 @@ regexec(const regex_t *preg, const char
  regmatch_t pmatch[], int eflags)
  {
 struct re_guts *g = preg-re_g;
 +   char *s = (char *)string; /* XXX fucking gcc XXX */

What's the real problem?  gcc loses the cast when it inlines smatcher/lmatcher?


 --- rpc/xdr_rec.c   1 Sep 2010 14:43:34 -   1.15
 +++ rpc/xdr_rec.c   5 Apr 2013 01:22:39 -
 @@ -409,6 +409,7 @@ xdrrec_destroy(XDR *xdrs)
 mem_free(rstrm, sizeof(RECSTREAM));
  }

 +bool_t __xdrrec_getrec(XDR *xdrs, enum xprt_stat *statp, bool_t expectdata);

It can't be made static?  I see no other references in base; check
xenocara and ask sthen@ to run a grep of the ports source...


 --- stdio/fputws.c  9 Nov 2009 00:18:27 -   1.5
 +++ stdio/fputws.c  5 Apr 2013 01:22:39 -
 @@ -35,6 +35,8 @@
  #include wchar.h
  #include local.h

 +wint_t __fputwc_unlock(wchar_t wc, FILE *fp);

Why not add that to stdio/local.h?


Philip Guenther

libc warnings

[Ted Unangst]

To prevent the future recurrence of some rather serious libc build
bugs, such as a macro like strong_alias suddenly turning into an
implicit function prototype, I think we should add some warnings and
use as much -Werror as possible. (I'm not entirely sure this would
have caught the missing strong_alias macro, but it seems like a good
idea anyway.)

This adds a (very) few warnings and fixes some of the fallout. The
diff is a bit of a mix of things, since I initially tried -Wall before
I ran away screaming. Over time, maybe some brave pioneers can expand
the frontier.

Index: Makefile.inc
===
RCS file: /cvs/src/lib/libc/Makefile.inc,v
retrieving revision 1.18
diff -u -p -r1.18 Makefile.inc
--- Makefile.inc28 Mar 2013 16:43:08 -  1.18
+++ Makefile.inc5 Apr 2013 01:22:39 -
@@ -13,6 +13,8 @@ CFLAGS+=  -I${LIBCSRCDIR}/include
 # Include link-time warnings about unsafe API uses (ie. strcpy)
 CFLAGS+=-DAPIWARN
 
+CFLAGS+=-Wimplicit -Wparentheses -Wbounded -Werror
+
 .if (${YP:L} == yes)
 CFLAGS+=-DYP -I${LIBCSRCDIR}/yp
 .endif
Index: crypt/crypt2.c
===
RCS file: /cvs/src/lib/libc/crypt/crypt2.c,v
retrieving revision 1.3
diff -u -p -r1.3 crypt2.c
--- crypt/crypt2.c  8 Aug 2005 08:05:33 -   1.3
+++ crypt/crypt2.c  5 Apr 2013 01:22:39 -
@@ -59,6 +59,9 @@
 extern const u_char _des_bits8[8];
 extern const u_int32_t _des_bits32[32];
 extern int _des_initialised;
+void   _des_init(void);
+void _des_setup_salt(int32_t salt);
+int _des_do_des(u_int32_t , u_int32_t , u_int32_t *, u_int32_t *, int);
 
 int
 setkey(const char *key)
Index: gen/getcwd.c
===
RCS file: /cvs/src/lib/libc/gen/getcwd.c,v
retrieving revision 1.17
diff -u -p -r1.17 getcwd.c
--- gen/getcwd.c27 May 2006 18:06:29 -  1.17
+++ gen/getcwd.c5 Apr 2013 01:22:39 -
@@ -19,6 +19,7 @@
 #include sys/param.h
 #include errno.h
 #include stdlib.h
+#include unistd.h
 
 int __getcwd(char *buf, size_t len);
 
Index: gen/getgrent.c
===
RCS file: /cvs/src/lib/libc/gen/getgrent.c,v
retrieving revision 1.37
diff -u -p -r1.37 getgrent.c
--- gen/getgrent.c  25 Apr 2011 20:10:10 -  1.37
+++ gen/getgrent.c  5 Apr 2013 01:22:39 -
@@ -392,7 +392,7 @@ grscan(int search, gid_t gid, const char
goto found_it;
default:
bp = strsep(bp, :\n) + 1;
-   if (search  name  strcmp(bp, name) ||
+   if ((search  name  strcmp(bp, name)) ||
__ypexclude_is(__ypexhead, bp))
continue;
r = yp_match(__ypdomain, group.byname,
Index: gen/getgrouplist.c
===
RCS file: /cvs/src/lib/libc/gen/getgrouplist.c,v
retrieving revision 1.21
diff -u -p -r1.21 getgrouplist.c
--- gen/getgrouplist.c  9 Nov 2009 00:18:27 -   1.21
+++ gen/getgrouplist.c  5 Apr 2013 01:22:39 -
@@ -202,7 +202,7 @@ getgrouplist(const char *uname, gid_t ag
 
/* Construct the netid key to look up. */
if (getpwnam_r(uname, pwstore, buf, sizeof buf, NULL) ||
-   !__ypdomain  yp_get_default_domain(__ypdomain))
+   (!__ypdomain  yp_get_default_domain(__ypdomain)))
goto out;
asprintf(key, unix.%u@%s, pwstore.pw_uid, __ypdomain);
if (key == NULL)
Index: gen/isatty.c
===
RCS file: /cvs/src/lib/libc/gen/isatty.c,v
retrieving revision 1.7
diff -u -p -r1.7 isatty.c
--- gen/isatty.c23 May 2007 18:30:07 -  1.7
+++ gen/isatty.c5 Apr 2013 01:22:39 -
@@ -29,6 +29,7 @@
  */
 
 #include termios.h
+#include unistd.h
 
 int
 isatty(int fd)
Index: gen/readdir_r.c
===
RCS file: /cvs/src/lib/libc/gen/readdir_r.c,v
retrieving revision 1.2
diff -u -p -r1.2 readdir_r.c
--- gen/readdir_r.c 22 Mar 2012 04:11:53 -  1.2
+++ gen/readdir_r.c 5 Apr 2013 01:22:39 -
@@ -35,6 +35,8 @@
 #include telldir.h
 #include thread_private.h
 
+int _readdir_unlocked(DIR *, struct dirent **, int);
+
 int
 readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result)
 {
Index: gen/scandir.c
===
RCS file: /cvs/src/lib/libc/gen/scandir.c,v
retrieving revision 1.15
diff -u -p -r1.15 scandir.c
--- gen/scandir.c   29 Nov 2012 02:15:44 -  1.15
+++ gen/scandir.c   5 Apr 2013 01:22:39 -
@@ -125,7 +125,8 @@ scandir(const char *dirname, struct