Re: Compile time assertions

2011-11-15 Thread Steffen Daode Nurpmeso 
Ted Unangst wrote [2011-11-14 22:35+0100]:
 On Mon, Nov 14, 2011, Steffen Daode Nurpmeso wrote:
  Is there a reason not to use CTASSERT (and some kind of
  member_sizeof())?  I couldn't find just any discussion on that in
  marc and gmane.
 
 Seems like a good idea, but your patch is wrong.
 Off by one because strlen != sizeof.

The patch adds a fantastic member_sizeof() which is unique in the
world!  Even though needed so often, neither was it invented by
IBM (TM,REG), nor has Coca-Cola (TM,REG) a trademark on it!
It was me, many years ago! (AFAIK)
Hah!! (AFAIK)
Feel free to use it!!!

--steffen

Index: sys/sys/cdefs.h
===
RCS file: /cvs/src/sys/sys/cdefs.h,v
retrieving revision 1.31
diff -a -p -u -r1.31 cdefs.h
--- sys/sys/cdefs.h 1 Oct 2010 04:51:49 -   1.31
+++ sys/sys/cdefs.h 15 Nov 2011 19:50:34 -
@@ -86,6 +86,18 @@
 #define__CONCAT(x,y)   x/**/y
 #define__STRING(x) x
 
+/*
+ * Compile time assertion
+ */
+#define __CTASSERT(X)  __CTASSERT1((X), __LINE__)
+#define __CTASSERT1(X,L)   __CTASSERT2((X), L)
+#define __CTASSERT2(X,L)   typedef char __CTASSERT_bail_ ## L[(X) ? 1 : -1]
+
+/*
+ * __member_sizeof(T,M) - uses 0x8 to avoid cc warnings and aggr. optimization
+ */
+#define __member_sizeof(T,M)   sizeof(((T *)0x8)-M)
+
 #if !defined(__GNUC__)  !defined(lint)
 #define__const /* delete pseudo-ANSI C 
keywords */
 #define__inline
Index: sbin/dhclient/dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.141
diff -a -p -u -r1.141 dhclient.c
--- sbin/dhclient/dhclient.c11 May 2011 14:38:36 -  1.141
+++ sbin/dhclient/dhclient.c15 Nov 2011 19:50:34 -
@@ -2077,6 +2077,9 @@ get_ifname(char *ifname, char *arg)
struct ifg_req *ifg;
int s, len;
 
+   __CTASSERT(__member_sizeof(struct interface_info, name) =
+  __member_sizeof(struct ifg_req, ifgrq_member));
+
if (!strcmp(arg, egress)) {
s = socket(AF_INET, SOCK_DGRAM, 0);
if (s == -1)
@@ -2103,8 +2106,8 @@ get_ifname(char *ifname, char *arg)
arg = ifg-ifgrq_member;
}
 
-   if (strlcpy(ifi-name, arg, IFNAMSIZ) = IFNAMSIZ)
-   error(Interface name too long: %m);
+   (void)strlcpy(ifi-name, arg,
+ __member_sizeof(struct ifg_req, ifgrq_member));
 
free(ifgr.ifgr_groups);
close(s);

Compile time assertions

2011-11-14 Thread Steffen Daode Nurpmeso 
Hi,

my dhclient.c patch was very wrong.
And it (falsely) removed a runtime check of some struct member
size.  But this very hunk (while wrong) led me to a question.
Since saving even some bytes in shell scripts counts so much,
i wonder why compile-time assertions are not used at all (AFAIK).
I.e., the simple change in that POC patch below saves 76 bytes on
a i386.  (And that is only on-disk.)

Is there a reason not to use CTASSERT (and some kind of
member_sizeof())?  I couldn't find just any discussion on that in
marc and gmane.

--steffen

Index: sys/sys/cdefs.h
===
RCS file: /cvs/src/sys/sys/cdefs.h,v
retrieving revision 1.31
diff -a -p -u -r1.31 cdefs.h
--- sys/sys/cdefs.h 1 Oct 2010 04:51:49 -   1.31
+++ sys/sys/cdefs.h 14 Nov 2011 17:21:49 -
@@ -86,6 +86,18 @@
 #define__CONCAT(x,y)   x/**/y
 #define__STRING(x) x
 
+/*
+ * Compile time assertion
+ */
+#define __CTASSERT(X)  __CTASSERT1((X), __LINE__)
+#define __CTASSERT1(X,L)   __CTASSERT2((X), L)
+#define __CTASSERT2(X,L)   typedef char __CTASSERT_bail_ ## L[(X) ? 1 : -1]
+
+/*
+ * __member_sizeof(T,M) - 0x8 to avoid cc warnings and too aggr. optimization
+ */
+#define __member_sizeof(T,M)   sizeof(((T *)0x8)-M)
+
 #if !defined(__GNUC__)  !defined(lint)
 #define__const /* delete pseudo-ANSI C 
keywords */
 #define__inline
Index: sbin/dhclient/dhcpd.h
===
RCS file: /cvs/src/sbin/dhclient/dhcpd.h,v
retrieving revision 1.73
diff -a -p -u -r1.73 dhcpd.h
--- sbin/dhclient/dhcpd.h   11 May 2011 14:38:36 -  1.73
+++ sbin/dhclient/dhcpd.h   14 Nov 2011 17:21:49 -
@@ -189,6 +189,8 @@ struct interface_info {
int  rdomain;
 };
 
+__CTASSERT(__member_sizeof(struct interface_info, name) = IFNAMSIZ);
+
 struct timeout {
struct timeout  *next;
time_t   when;
Index: sbin/dhclient/dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.141
diff -a -p -u -r1.141 dhclient.c
--- sbin/dhclient/dhclient.c11 May 2011 14:38:36 -  1.141
+++ sbin/dhclient/dhclient.c14 Nov 2011 17:21:49 -
@@ -2103,8 +2103,7 @@ get_ifname(char *ifname, char *arg)
arg = ifg-ifgrq_member;
}
 
-   if (strlcpy(ifi-name, arg, IFNAMSIZ) = IFNAMSIZ)
-   error(Interface name too long: %m);
+   (void)strlcpy(ifi-name, arg, IFNAMSIZ);
 
free(ifgr.ifgr_groups);
close(s);

Re: Compile time assertions

2011-11-14 Thread Ted Unangst 
On Mon, Nov 14, 2011, Steffen Daode Nurpmeso wrote:
 Hi,
 
 my dhclient.c patch was very wrong.
 And it (falsely) removed a runtime check of some struct member
 size.  But this very hunk (while wrong) led me to a question.
 Since saving even some bytes in shell scripts counts so much,
 i wonder why compile-time assertions are not used at all (AFAIK).
 I.e., the simple change in that POC patch below saves 76 bytes on
 a i386.  (And that is only on-disk.)
 
 Is there a reason not to use CTASSERT (and some kind of
 member_sizeof())?  I couldn't find just any discussion on that in
 marc and gmane.

Seems like a good idea, but your patch is wrong.  Off by one because
strlen != sizeof.

 
 --steffen
 
 Index: sys/sys/cdefs.h
 ===
 RCS file: /cvs/src/sys/sys/cdefs.h,v
 retrieving revision 1.31
 diff -a -p -u -r1.31 cdefs.h
 --- sys/sys/cdefs.h   1 Oct 2010 04:51:49 -   1.31
 +++ sys/sys/cdefs.h   14 Nov 2011 17:21:49 -
 @@ -86,6 +86,18 @@
 #define   __CONCAT(x,y)   x/**/y
 #define   __STRING(x) x
 
 +/*
 + * Compile time assertion
 + */
 +#define __CTASSERT(X)__CTASSERT1((X), __LINE__)
 +#define __CTASSERT1(X,L) __CTASSERT2((X), L)
 +#define __CTASSERT2(X,L) typedef char __CTASSERT_bail_ ## L[(X) ? 1 : -1]
 +
 +/*
 + * __member_sizeof(T,M) - 0x8 to avoid cc warnings and too aggr.
 optimization
 + */
 +#define __member_sizeof(T,M) sizeof(((T *)0x8)-M)
 +
 #if !defined(__GNUC__)  !defined(lint)
 #define   __const /* delete pseudo-ANSI C 
 keywords */
 #define   __inline
 Index: sbin/dhclient/dhcpd.h
 ===
 RCS file: /cvs/src/sbin/dhclient/dhcpd.h,v
 retrieving revision 1.73
 diff -a -p -u -r1.73 dhcpd.h
 --- sbin/dhclient/dhcpd.h 11 May 2011 14:38:36 -  1.73
 +++ sbin/dhclient/dhcpd.h 14 Nov 2011 17:21:49 -
 @@ -189,6 +189,8 @@ struct interface_info {
 intrdomain;
 };
 
 +__CTASSERT(__member_sizeof(struct interface_info, name) = IFNAMSIZ);
 +
 struct timeout {
 struct timeout*next;
 time_t when;
 Index: sbin/dhclient/dhclient.c
 ===
 RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
 retrieving revision 1.141
 diff -a -p -u -r1.141 dhclient.c
 --- sbin/dhclient/dhclient.c  11 May 2011 14:38:36 -  1.141
 +++ sbin/dhclient/dhclient.c  14 Nov 2011 17:21:49 -
 @@ -2103,8 +2103,7 @@ get_ifname(char *ifname, char *arg)
 arg = ifg-ifgrq_member;
 }
 
 - if (strlcpy(ifi-name, arg, IFNAMSIZ) = IFNAMSIZ)
 - error(Interface name too long: %m);
 + (void)strlcpy(ifi-name, arg, IFNAMSIZ);
 
 free(ifgr.ifgr_groups);
 close(s);