Re: mbstowcs() null termination

2012-03-30 Thread Steffen Daode Nurpmeso
Hi,

Yarin wrote [2012-03-30 17:01+0200]:
 Hello,
 
 Is mbstowcs() suppose to null-terminate? I ask because, on OpenBSD 4.9 
 (generic, no patches), it never null terminates.
 Even though the C90 draft seems to imply that it should when there's enough 
 room.
 
 http://www.open-std.org/jtc1/sc22/wg14/www/docs/n869/
 
 I'm unsure of the expected behavior here, and wasn't able to find anything on 
 the web, which is why I ask this, rather than submit a bug report.
 
 Thanks

I like the POSIX stuff better from within the browser:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/mbstowcs.html
(or http://pubs.opengroup.org/onlinepubs/9699919799/ to get the
full frameset and then search for the function).

The relevant OpenBSD code seems to be in
src/lib/libc/citrus/citrus_{none,utf8}.c and behaves correctly for
a small test like this linked against -current:

  #include stdio.h
  #include stdlib.h
  #include wchar.h

  int
  main(void)
  {
wchar_t dst[16];
size_t res;

(void)wmemset(dst, (wchar_t)-1, 16);
res = mbstowcs(dst, CuCaRaZa, 16);
printf(len=%zu, TERM=%d\n, res, dst[res] == 0);

(void)wmemset(dst, (wchar_t)-1, 16);
res = mbstowcs(dst, CuCaRaZa, 8);
printf(len=%zu, TERM=%d\n, res, dst[res] == 0);

return (0);
  }

  ?0%1[steffen@obsdc src]$ cc -o zt test.c
  ?0%1[steffen@obsdc src]$ ./zt 
  len=8, TERM=1
  len=8, TERM=0

It seems that the UTF-8 version had a fix after tag 4.9 had been
applied, and that has not been backported into 4.9.
From what i see at a glance invalid sequence errors could have
occurred due to usage of the false length parameter.

But say - UTF-8 locales were pretty cutting edge then .. ?

--steffen
Forza Figa!



Re: cvs, stop ignoring *@#%@# files/directories named core

2012-03-26 Thread Steffen Daode Nurpmeso
Stuart Henderson wrote [2012-03-26 11:16+0200]:
 An addition to ignore .git files, some of us use git locally to track
 changes pre-commit and, just like the existing ignore entries for
 RCS/SCCS/CVS control files, there is no reason to import these to CVS
 without an explicit ignore.

Yeah!
Adding .gitignore in addition would be fine, then.

--steffen
Forza Figa!



wait4() busy endless OR: gcc: Internal error: Trace/BPT trap (program cc1)

2012-03-19 Thread Steffen Daode Nurpmeso
Sorry folks for this shitty report, but i thought it's maybe
better to send it out than not to be able to sleep or so.
I guess it's somehow related to the rthread wait4() path, but of
course i dunno.  Here's what's happened:

As always i did my weekly kernel+userland compilation this noon,
and i was running in an endless loop in wait4() when compiling
insn-attrtab.i (thought preprocessor hung, but it's later).

Note i (yet) use(d) a very primitive compilation approach
- i simply do 'make all', which sometimes requires changes by hand
(e.g. today drop of SO_JUMBO in kdump).  *This* is only a fellow
traveler virtual box, and doing the full rebuild blocks normal
work out some hours longer, so..
I had not yet time to check this on a real box.

Anyway, kernel compilation was successful, and (after reboot) much
of userland (e.g. libC, crypto etc. - one week of work), too.
But then i ran in an endless (busy) wait4() upon compilation of
gnu/usr.bin/cc/cc_int/obj/insn-attrtab:

   68 -rw---  1 steffen  wsrc 68981 Mar 19 16:34 ktrace.out

  1844 gcc  NAMI  /usr/lib/gcc-lib/amd64-unknown-openbsd5.1/4.2.1/cc1
  1844 gcc  RET   access 0
  1844 gcc  CALL  sigprocmask(SIG_BLOCK,~0)
  1844 gcc  RET   sigprocmask 0
  1844 gcc  CALL  mprotect(0x72f000,0x2000,0x3PROT_READ|PROT_WRITE)
  1844 gcc  RET   mprotect 0
  1844 gcc  CALL  mprotect(0x72f000,0x2000,0x1PROT_READ)
  1844 gcc  RET   mprotect 0
  1844 gcc  CALL  sigprocmask(SIG_SETMASK,0)
  1844 gcc  RET   sigprocmask ~0x10100SIGKILL|SIGSTOP
  1844 gcc  CALL  vfork()
  1844 gcc  RET   vfork 24913/0x6151
  1844 gcc  CALL  sigprocmask(SIG_BLOCK,~0)
  1844 gcc  RET   sigprocmask 0
  1844 gcc  CALL  mprotect(0x72f000,0x2000,0x3PROT_READ|PROT_WRITE)
  1844 gcc  RET   mprotect 0
  1844 gcc  CALL  mprotect(0x72f000,0x2000,0x1PROT_READ)  1844 gcc  
RET   mprotect 0
  1844 gcc  CALL  sigprocmask(SIG_SETMASK,0)
  1844 gcc  RET   sigprocmask ~0x10100SIGKILL|SIGSTOP
  1844 gcc  CALL  sigprocmask(SIG_BLOCK,~0)
  1844 gcc  RET   sigprocmask 0
  1844 gcc  CALL  mprotect(0x20c61,0x2000,0x3PROT_READ|PROT_WRITE)
  1844 gcc  RET   mprotect 0
  1844 gcc  CALL  mprotect(0x20c61,0x2000,0x1PROT_READ)
  1844 gcc  RET   mprotect 0
  1844 gcc  CALL  sigprocmask(SIG_SETMASK,0)
  1844 gcc  RET   sigprocmask ~0x10100SIGKILL|SIGSTOP
  1844 gcc  CALL  wait4(0x6151,0x208e980d0,0,0)
 ENDLESS BUSY LOOP, ^C HERE 
  1844 gcc  PSIG  SIGINT caught handler=0x40b210 mask=0
  1844 gcc  RET   wait4 RESTART
  1844 gcc  CALL  sigaction(SIGINT,0x7f7f0e10,0x7f7f0e00)

I've moved the fresh libC to /usr/lib, but that results in the
very same problem.  Ditto last week's kernel.
Puke!  I had to run a debugger :-((:

  (gdb) break vfork
  Breakpoint 1 at 0x401a50
  (gdb) run
  Starting program: /usr/bin/gcc -O2 -DIN_GCC -DHAVE_CONFIG_H -DPREFIX=/usr 
-I/home/src/gnu/usr.bin/cc/cc_int/obj/../cc_tools 
-I/home/src/gnu/usr.bin/cc/cc_int/../cc_tools 
-I/home/src/gnu/usr.bin/cc/cc_int/../../../gcc/gcc 
-I/home/src/gnu/usr.bin/cc/cc_int/../../../gcc/gcc/config 
-I/home/src/gnu/usr.bin/cc/cc_int/../../../gcc/include 
-I/home/src/gnu/usr.bin/cc/cc_int/../../../gcc/libcpp/include 
-I/home/src/gnu/usr.bin/cc/cc_int/../../../gcc/libdecnumber -c 
../cc_tools/insn-attrtab.c -o insn-attrtab.o.o
  Breakpoint 1 at 0x2088b0f90

  Breakpoint 1, 0x0002088b0f90 in vfork () from /usr/lib/libc.so.62.0
  (gdb) step
  Single stepping until exit from function vfork, 
  which has no line number information.
  gcc: Internal error: Trace/BPT trap (program cc1)
  Please submit a full bug report.
  See URL:http://gcc.gnu.org/bugs.html for instructions.

  Program exited with code 01.

The old libc.so.62.0 is from monday last week, the new from today.
I'll append a dmesg of this VirtualBox.

Well; unless there is interest in something i'll do a complete
cleanup and recompile tomorrow.
I would report if the problem remains, then.
(I use my own homebrew environment since a decade, and have no
experiences with - spew - debuggers.  Just for info.)

--steffen
Forza Figa!

OpenBSD 5.1-current (GENERIC) #13: Mon Mar 19 12:13:01 CET 2012
sdaoden@obsd.sherwood:/home/src/sys/arch/amd64/compile/GENERIC
real mem = 133103616 (126MB)
avail mem = 107405312 (102MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.5 @ 0xe1000 (5 entries)
bios0: vendor innotek GmbH version VirtualBox date 12/01/2006
bios0: innotek GmbH VirtualBox
acpi0 at bios0: rev 2
acpi0: sleep states S0 S5
acpi0: tables DSDT FACP APIC SSDT
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM)2 Duo CPU P7550 @ 2.26GHz, 1974.41 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,MWAIT,SSSE3,NXE,LONG,LAHF
cpu0: 3MB 64b/line 8-way L2 cache
cpu0: apic clock running 

Re: vmmap replacement -- please test

2012-03-02 Thread Steffen Daode Nurpmeso
Hi,

David Coppa wrote:
 still rocking hard

My old Athlon 1600+ blow dries something like

  I'm gruvm up the country, where the memory tastes like wine.

Is this a regression, then??

--steffen



Re: -not for find(1)

2012-02-28 Thread Steffen Daode Nurpmeso
(peep)



-not for find(1)

2012-02-27 Thread Steffen Daode Nurpmeso
Hello,

this patch adds the -not operator to find(1).
I personally always found -not easier to use due to shell
escaping, but today may laziness has bitten back.
And it's just one more non-POSIX-compliant option.

--steffen

Index: usr.bin/find/find.1
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.bin/find/find.1,v
retrieving revision 1.85
diff -a -p -u -r1.85 find.1
--- usr.bin/find/find.1 5 Jan 2012 13:16:10 -   1.85
+++ usr.bin/find/find.1 27 Feb 2012 18:11:14 -
@@ -520,6 +520,7 @@ This evaluates to true if the parenthesi
 true.
 .Pp
 .It Cm \! Ar expression
+.It Cm -not Ar expression
 This is the unary
 .Tn NOT
 operator.
@@ -607,8 +608,8 @@ utility is compliant with the
 specification.
 .Pp
 The options
-.Op Fl dfhXx
-and primaries
+.Op Fl dfhXx ,
+primaries
 .Ic -amin ,
 .Ic -anewer ,
 .Ic -cmin ,
@@ -626,6 +627,8 @@ and primaries
 .Ic -mmin ,
 and
 .Ic -print0
+and the operator
+.Ic Cm -not
 are extensions to that specification.
 The
 .Ic -iname
Index: usr.bin/find/function.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.bin/find/function.c,v
retrieving revision 1.38
diff -a -p -u -r1.38 function.c
--- usr.bin/find/function.c 5 Jan 2012 10:21:33 -   1.38
+++ usr.bin/find/function.c 27 Feb 2012 17:58:28 -
@@ -1542,7 +1542,7 @@ c_closeparen(char *ignore, char ***ignor
 }
  
 /*
- * ! expression functions --
+ * ! expression/-not expression functions --
  *
  * Negation of a primary; the unary NOT operator.
  */
Index: usr.bin/find/option.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.bin/find/option.c,v
retrieving revision 1.18
diff -a -p -u -r1.18 option.c
--- usr.bin/find/option.c   27 Oct 2009 23:59:38 -  1.18
+++ usr.bin/find/option.c   27 Feb 2012 17:57:45 -
@@ -79,6 +79,7 @@ static OPTION options[] = {
{ -name,  N_NAME, c_name, O_ARGV },
{ -newer, N_NEWER,c_newer,O_ARGV },
{ -nogroup,   N_NOGROUP,  c_nogroup,  O_ZERO },
+   { -not,   N_NOT,  c_not,  O_ZERO },
{ -nouser,N_NOUSER,   c_nouser,   O_ZERO },
{ -o, N_OR,   c_or,   O_ZERO },
{ -ok,N_OK,   c_exec, O_ARGVP },



Re: NEW: libc getdelim(3) and getline(3)

2012-02-26 Thread Steffen Daode Nurpmeso
pax(1).

--steffen
 
Index: bin/pax/options.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/bin/pax/options.c,v
retrieving revision 1.74
diff -a -p -u -r1.74 options.c
--- bin/pax/options.c   2 Dec 2010 04:08:27 -   1.74
+++ bin/pax/options.c   26 Feb 2012 17:06:00 -
@@ -64,7 +64,7 @@ static int no_op(void);
 static void printflg(unsigned int);
 static int c_frmt(const void *, const void *);
 static off_t str_offt(char *);
-static char *getline(FILE *fp);
+static char *pax_getline(FILE *fp);
 static void pax_options(int, char **);
 static void pax_usage(void);
 static void tar_options(int, char **);
@@ -72,10 +72,10 @@ static void tar_usage(void);
 static void cpio_options(int, char **);
 static void cpio_usage(void);
 
-/* errors from getline */
-#define GETLINE_FILE_CORRUPT 1
-#define GETLINE_OUT_OF_MEM 2
-static int getline_error;
+/* errors from pax_getline */
+#define PAX_GETLINE_FILE_CORRUPT   1
+#define PAX_GETLINE_OUT_OF_MEM 2
+static int pax_getline_error;
 
 
 #define GZIP_CMD   gzip  /* command to run as gzip */
@@ -884,14 +884,14 @@ tar_options(int argc, char **argv)
paxwarn(1, Unable to open file 
'%s' for read, file);
tar_usage();
}
-   while ((str = getline(fp)) != NULL) {
+   while ((str = pax_getline(fp)) != NULL) 
{
if (pat_add(str, dir)  0)
tar_usage();
sawpat = 1;
}
if (strcmp(file, -) != 0)
fclose(fp);
-   if (getline_error) {
+   if (pax_getline_error) {
paxwarn(1, Problem with file 
'%s', file);
tar_usage();
}
@@ -963,13 +963,13 @@ tar_options(int argc, char **argv)
paxwarn(1, Unable to open file '%s' 
for read, file);
tar_usage();
}
-   while ((str = getline(fp)) != NULL) {
+   while ((str = pax_getline(fp)) != NULL) {
if (ftree_add(str, 0)  0)
tar_usage();
}
if (strcmp(file, -) != 0)
fclose(fp);
-   if (getline_error) {
+   if (pax_getline_error) {
paxwarn(1, Problem with file '%s',
file);
tar_usage();
@@ -1185,11 +1185,11 @@ cpio_options(int argc, char **argv)
paxwarn(1, Unable to open file '%s' 
for read, optarg);
cpio_usage();
}
-   while ((str = getline(fp)) != NULL) {
+   while ((str = pax_getline(fp)) != NULL) {
pat_add(str, NULL);
}
fclose(fp);
-   if (getline_error) {
+   if (pax_getline_error) {
paxwarn(1, Problem with file '%s', 
optarg);
cpio_usage();
}
@@ -1284,10 +1284,10 @@ cpio_options(int argc, char **argv)
 * no read errors allowed on updates/append operation!
 */
maxflt = 0;
-   while ((str = getline(stdin)) != NULL) {
+   while ((str = pax_getline(stdin)) != NULL) {
ftree_add(str, 0);
}
-   if (getline_error) {
+   if (pax_getline_error) {
paxwarn(1, Problem while reading stdin);
cpio_usage();
}
@@ -1515,21 +1515,21 @@ str_offt(char *val)
 }
 
 char *
-getline(FILE *f)
+pax_getline(FILE *f)
 {
char *name, *temp;
size_t len;
 
name = fgetln(f, len);
if (!name) {
-   getline_error = ferror(f) ? GETLINE_FILE_CORRUPT : 0;
+   pax_getline_error = ferror(f) ? 

Re: NEW: libc getdelim(3) and getline(3)

2012-02-26 Thread Steffen Daode Nurpmeso
unifdef(1).
(Should complete bin and usr.bin.)
(Did i press 'g' all the time?  Sorry if not!)

--steffen

Index: usr.bin/unifdef/unifdef.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.bin/unifdef/unifdef.c,v
retrieving revision 1.14
diff -a -p -u -r1.14 unifdef.c
--- usr.bin/unifdef/unifdef.c   27 Oct 2009 23:59:46 -  1.14
+++ usr.bin/unifdef/unifdef.c   26 Feb 2012 17:16:50 -
@@ -183,7 +183,7 @@ static void debug(const char
 static void error(const char *);
 static int  findsym(const char *);
 static void flushline(bool);
-static Linetype getline(void);
+static Linetype ltgetline(void);
 static Linetype ifeval(const char **);
 static void ignoreoff(void);
 static void ignoreon(void);
@@ -663,7 +663,7 @@ process(void)
 
for (;;) {
linenum++;
-   lineval = getline();
+   lineval = ltgetline();
trans = trans_table[ifstate[depth]][lineval];
if (trans == NULL)
break;
@@ -681,7 +681,7 @@ process(void)
  * parser state between calls in a global variable.
  */
 static Linetype
-getline(void)
+ltgetline(void)
 {
const char *cp;
int cursym;



Re: NEW: libc getdelim(3) and getline(3)

2012-02-26 Thread Steffen Daode Nurpmeso
(All compile-tested after applying the patches.)

--steffen



ksh(1), smaller one

2012-02-25 Thread Steffen Daode Nurpmeso
And this smaller diff excludes the node-based table.

Since revision 1.8 (extra sanity checking for afree()) is the
youngest commit of alloc.c i guess that checking should not be
removed.  But that unidirectional list walk is a real killer:
with the test from one of my former mails and 1 variables this
results in 282.724.854 list-node walks, with 21000 variables it
results in 1.228.967.558 list-walks.  (And not joking!)

So use a hashtable to manage allocations of and in struct Area, and
only use singly-linked nodes therein, since the most time-critical
part can is performed with unidirectional walks anyway.  This
results in a faster execution with a smaller memory footprint.

(It must be said that the following benchmarks use the long
variable prefix.  But struct link is now only one pointer, so the
memory overhead of the node-based table is eliminated.)

Benchmarks, original, 1 variables:

?0%1[steffen@obsd src]$ /home/src/bin/ksh/obj/ksh tests/table-test.sh
0m1.97s real 0m1.86s user 0m0.06s system
0m1.47s real 0m1.40s user 0m0.03s system
0m1.47s real 0m1.44s user 0m0.01s system
init:2010144, freeall:2100161, alloc:9206483, resize:2760301,
free:4905640 (walks:282724854)

29603 R+55.0  2628  2368 ttyp1

Benchmarks, patched (also using hashmap in hashtable), 1 variables:

?0%0[steffen@obsd ksh]$ ./ksh tests/table-test.sh
0m1.37s real 0m1.32s user 0m0.04s system
0m1.28s real 0m1.25s user 0m0.01s system
0m1.27s real 0m1.20s user 0m0.05s system
init:2010144, freeall:2100161, alloc:9206483, resize:2760301,
free:4905636 (walks:39262535)
...
free:4905636 (walks:39260263)
...
free:4905636 (walks:39260811)

16713 R+84.7  2392  2152 ttyp0

Benchmarks, original, 21000 variables:

?0%1[steffen@obsd src]$ /home/src/bin/ksh/obj/ksh tests/table-test.sh
0m14.30s real 0m13.61s user 0m0.19s system
0m4.30s real 0m4.19s user 0m0.04s system
0m4.35s real 0m4.17s user 0m0.07s system
init:4221144, freeall:4410161, alloc:19331984, resize:5796301,
free:10301141 (walks:1228967558)

13799 R+97.4  4568  4196 ttyp1

Benchmarks, patched (also using hashmap in hashtable), 21000 variables:

?0%0[sdaoden@obsd ksh]$ ./ksh tests/table-test.sh
0m4.32s real 0m3.87s user 0m0.15s system
0m3.09s real 0m2.93s user 0m0.01s system
0m3.10s real 0m3.04s user 0m0.02s system
init:4221144, freeall:4410161, alloc:19331984, resize:5796301,
free:10301137 (walks:242558821)  - HASHMAP: 5 slots
...
free:10301137 (walks:242561158)  - HASHMAP: 5 slots
...
(free:10301137 (walks:173195475)  - HASHMAP: 7 slots)

11285 R+91.1  3980  3696 ttyp0

With a hashmap of seven slots one page more is shown by ps(1),
but for normal use cases five slots seem sufficient.
Also struct Area ends up with 40-bytes on 64-bit, not 56 as it
would otherwise.

--steffen

diff --git a/alloc.c b/alloc.c
index 7e41c2c..7e986a0 100644
--- a/alloc.c
+++ b/alloc.c
@@ -30,28 +30,35 @@
 
 #include sh.h
 
+#define ALLOCIDX(AP,P) ((size_t)(P) % NELEM((AP)-allocs))
+
 struct link {
-   struct link *prev;
struct link *next;
 };
 
 Area *
 ainit(Area *ap)
 {
-   ap-freelist = NULL;
-   return ap;
+   bzero(ap-allocs, sizeof(ap-allocs));
+   return (ap);
 }
 
 void
 afreeall(Area *ap)
 {
-   struct link *l, *l2;
+   size_t i;
+
+   for (i = NELEM(ap-allocs); i-- != 0;) {
+   struct link *l = ap-allocs[i];
+   ap-allocs[i] = NULL;
 
-   for (l = ap-freelist; l != NULL; l = l2) {
-   l2 = l-next;
-   free(l);
+   while (l != NULL) {
+   struct link *l2 = l;
+   l = l-next;
+   free(l2);
+   }
}
-   ap-freelist = NULL;
+   return;
 }
 
 #define L2P(l) ( (void *)(((char *)(l)) + sizeof(struct link)) )
@@ -60,68 +67,72 @@ afreeall(Area *ap)
 void *
 alloc(size_t size, Area *ap)
 {
-   struct link *l;
+   struct link *l, **arr;
 
l = malloc(sizeof(struct link) + size);
if (l == NULL)
internal_errorf(1, unable to allocate memory);
-   l-next = ap-freelist;
-   l-prev = NULL;
-   if (ap-freelist)
-   ap-freelist-prev = l;
-   ap-freelist = l;
 
-   return L2P(l);
+   arr = ap-allocs + ALLOCIDX(ap, l);
+   l-next = *arr;
+   *arr = l;
+
+   return (L2P(l));
 }
 
 void *
 aresize(void *ptr, size_t size, Area *ap)
 {
-   struct link *l, *l2, *lprev, *lnext;
+   struct link *l, **arr;
 
if (ptr == NULL)
return alloc(size, ap);
 
l = P2L(ptr);
-   lprev = l-prev;
-   lnext = l-next;
 
-   l2 = realloc(l, sizeof(struct link) + size);
-   if (l2 == NULL)
+   arr = ap-allocs + ALLOCIDX(ap, l);
+   if (*arr == l)
+   

ksh(1), big one

2012-02-25 Thread Steffen Daode Nurpmeso
Wanted or not, here is the promised big diff which does the
following:

1. Encapsulate struct table iteration usage
   (*All* direct accesses encapsulated with struct tstate.)
2. ktinit() shouldn't expect pow2 size requests..
   (It's really better that way.)
3. Turn struct table over to node-based approach
   (Tight and sultry - or do you prefer Route 66.)
4. Use a hashmap for struct Area managed allocs..
   (Blow my little white ass right off the map!
   And turn the mentioned endless loop into something that is
   supersafe and of at least acceptable performance.)

1, 2 and 3 survived a week of work (including one kernel
compilation, and one make build-alike).
4 is from friday evening.

--steffen

diff --git a/alloc.c b/alloc.c
index 7e41c2c..7e986a0 100644
--- a/alloc.c
+++ b/alloc.c
@@ -30,28 +30,35 @@
 
 #include sh.h
 
+#define ALLOCIDX(AP,P) ((size_t)(P) % NELEM((AP)-allocs))
+
 struct link {
-   struct link *prev;
struct link *next;
 };
 
 Area *
 ainit(Area *ap)
 {
-   ap-freelist = NULL;
-   return ap;
+   bzero(ap-allocs, sizeof(ap-allocs));
+   return (ap);
 }
 
 void
 afreeall(Area *ap)
 {
-   struct link *l, *l2;
+   size_t i;
+
+   for (i = NELEM(ap-allocs); i-- != 0;) {
+   struct link *l = ap-allocs[i];
+   ap-allocs[i] = NULL;
 
-   for (l = ap-freelist; l != NULL; l = l2) {
-   l2 = l-next;
-   free(l);
+   while (l != NULL) {
+   struct link *l2 = l;
+   l = l-next;
+   free(l2);
+   }
}
-   ap-freelist = NULL;
+   return;
 }
 
 #define L2P(l) ( (void *)(((char *)(l)) + sizeof(struct link)) )
@@ -60,68 +67,72 @@ afreeall(Area *ap)
 void *
 alloc(size_t size, Area *ap)
 {
-   struct link *l;
+   struct link *l, **arr;
 
l = malloc(sizeof(struct link) + size);
if (l == NULL)
internal_errorf(1, unable to allocate memory);
-   l-next = ap-freelist;
-   l-prev = NULL;
-   if (ap-freelist)
-   ap-freelist-prev = l;
-   ap-freelist = l;
 
-   return L2P(l);
+   arr = ap-allocs + ALLOCIDX(ap, l);
+   l-next = *arr;
+   *arr = l;
+
+   return (L2P(l));
 }
 
 void *
 aresize(void *ptr, size_t size, Area *ap)
 {
-   struct link *l, *l2, *lprev, *lnext;
+   struct link *l, **arr;
 
if (ptr == NULL)
return alloc(size, ap);
 
l = P2L(ptr);
-   lprev = l-prev;
-   lnext = l-next;
 
-   l2 = realloc(l, sizeof(struct link) + size);
-   if (l2 == NULL)
+   arr = ap-allocs + ALLOCIDX(ap, l);
+   if (*arr == l)
+   *arr = l-next;
+   else {
+   struct link *p = *arr;
+   while (p-next != l)
+   p = p-next;
+   p-next = l-next;
+   }
+
+   l = realloc(l, sizeof(struct link) + size);
+   if (l == NULL)
internal_errorf(1, unable to allocate memory);
-   if (lprev)
-   lprev-next = l2;
-   else
-   ap-freelist = l2;
-   if (lnext)
-   lnext-prev = l2;
-
-   return L2P(l2);
+
+   arr = ap-allocs + ALLOCIDX(ap, l);
+   l-next = *arr;
+   *arr = l;
+
+   return (L2P(l));
 }
 
 void
 afree(void *ptr, Area *ap)
 {
-   struct link *l, *l2;
+   struct link *l, **arr;
 
if (!ptr)
return;
-
l = P2L(ptr);
 
-   for (l2 = ap-freelist; l2 != NULL; l2 = l2-next) {
-   if (l == l2)
-   break;
+   arr = ap-allocs + ALLOCIDX(ap, l);
+   if (*arr == l)
+   *arr = l-next;
+   else {
+   struct link *p = *arr;
+   while (p != NULL  p-next != l)
+   p = p-next;
+   if (p == NULL)
+   internal_errorf(1, afree: %p not present in area %p,
+   ptr, (void*)ap);
+   p-next = l-next;
}
-   if (l2 == NULL)
-   internal_errorf(1, afree: %p not present in area %p, ptr, ap);
-
-   if (l-prev)
-   l-prev-next = l-next;
-   else
-   ap-freelist = l-next;
-   if (l-next)
-   l-next-prev = l-prev;
 
free(l);
+   return;
 }
diff --git a/exec.c b/exec.c
index 1c144fa..fb1e5bb 100644
--- a/exec.c
+++ b/exec.c
@@ -920,11 +920,14 @@ flushcom(int all) /* just relative or all */
struct tbl *tp;
struct tstate ts;
 
-   for (ktwalk(ts, taliases); (tp = ktnext(ts)) != NULL; )
-   if ((tp-flagISSET)  (all || tp-val.s[0] != '/')) {
+   ktwalk(ts, taliases);
+   ts.flag |= ISSET;
+   while ((tp = ktnext(ts)) != NULL)
+   if (all || tp-val.s[0] != '/') {
if (tp-flagALLOC) {
tp-flag = ~(ALLOC|ISSET);
afree(tp-val.s, APERM);

Re: NEW: libc getdelim(3) and getline(3)

2012-02-25 Thread Steffen Daode Nurpmeso
uudecode(1).

--steffen

Index: usr.bin/uudecode/uudecode.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.bin/uudecode/uudecode.c,v
retrieving revision 1.17
diff -a -p -u -r1.17 uudecode.c
--- usr.bin/uudecode/uudecode.c 27 Oct 2009 23:59:46 -  1.17
+++ usr.bin/uudecode/uudecode.c 25 Feb 2012 13:36:51 -
@@ -304,7 +304,7 @@ decode2(void)
 }
 
 static int
-getline(char *buf, size_t size)
+uugetline(char *buf, size_t size)
 {
if (fgets(buf, size, infp) != NULL)
return (2);
@@ -341,7 +341,7 @@ uu_decode(void)
 
/* for each input line */
for (;;) {
-   switch (getline(buf, sizeof(buf))) {
+   switch (uugetline(buf, sizeof(buf))) {
case 0:
return (0);
case 1:
@@ -401,7 +401,7 @@ uu_decode(void)
}
}
}
-   switch (getline(buf, sizeof(buf))) {
+   switch (uugetline(buf, sizeof(buf))) {
case 0:
return (0);
case 1:
@@ -419,7 +419,7 @@ base64_decode(void)
unsigned char outbuf[MAXPATHLEN * 4];
 
for (;;) {
-   switch (getline(inbuf, sizeof(inbuf))) {
+   switch (uugetline(inbuf, sizeof(inbuf))) {
case 0:
return (0);
case 1:



Re: NEW: libc getdelim(3) and getline(3)

2012-02-25 Thread Steffen Daode Nurpmeso
lpr(1).

--steffen

Index: usr.sbin/lpr/common_source/common.c
===
RCS file: 
/Users/steffen/arena/code.openbsd/src/usr.sbin/lpr/common_source/common.c,v
retrieving revision 1.33
diff -a -p -u -r1.33 common.c
--- usr.sbin/lpr/common_source/common.c 27 Oct 2009 23:59:51 -  1.33
+++ usr.sbin/lpr/common_source/common.c 25 Feb 2012 13:33:26 -
@@ -199,7 +199,7 @@ retryport:
  * Returns 0 at EOF or the number of characters read.
  */
 int
-getline(FILE *cfp)
+lpgetline(FILE *cfp)
 {
int linel = 0;
char *lp = line;
Index: usr.sbin/lpr/common_source/displayq.c
===
RCS file: 
/Users/steffen/arena/code.openbsd/src/usr.sbin/lpr/common_source/displayq.c,v
retrieving revision 1.31
diff -a -p -u -r1.31 displayq.c
--- usr.sbin/lpr/common_source/displayq.c   27 Oct 2009 23:59:51 -  
1.31
+++ usr.sbin/lpr/common_source/displayq.c   25 Feb 2012 13:40:04 -
@@ -352,7 +352,7 @@ inform(char *cf, int rank)
}
 
j = 0;
-   while (getline(cfp)) {
+   while (lpgetline(cfp)) {
switch (line[0]) {
case 'P': /* Was this file specified in the user's list? */
if (!inlist(line+1, cf)) {
Index: usr.sbin/lpr/common_source/lp.h
===
RCS file: 
/Users/steffen/arena/code.openbsd/src/usr.sbin/lpr/common_source/lp.h,v
retrieving revision 1.14
diff -a -p -u -r1.14 lp.h
--- usr.sbin/lpr/common_source/lp.h 2 Jun 2003 23:36:53 -   1.14
+++ usr.sbin/lpr/common_source/lp.h 25 Feb 2012 13:34:08 -
@@ -130,7 +130,7 @@ void displayq(int);
 void dump(char *, char *, int);
 __dead void fatal(const char *, ...)
__attribute__((__format__(__printf__, 1, 2)));
-int getline(FILE *);
+int lpgetline(FILE *);
 int getport(char *, int);
 int getq(struct queue ***);
 void header(void);
Index: usr.sbin/lpr/common_source/rmjob.c
===
RCS file: 
/Users/steffen/arena/code.openbsd/src/usr.sbin/lpr/common_source/rmjob.c,v
retrieving revision 1.18
diff -a -p -u -r1.18 rmjob.c
--- usr.sbin/lpr/common_source/rmjob.c  27 Oct 2009 23:59:51 -  1.18
+++ usr.sbin/lpr/common_source/rmjob.c  25 Feb 2012 13:33:55 -
@@ -171,7 +171,7 @@ lockchk(char *s)
else
return(0);
}
-   if (!getline(fp)) {
+   if (!lpgetline(fp)) {
(void)fclose(fp);
return(0);  /* no daemon present */
}
@@ -211,7 +211,7 @@ process(char *file)
close(fd);
fatal(cannot open %s, file);
}
-   while (getline(cfp)) {
+   while (lpgetline(cfp)) {
switch (line[0]) {
case 'U':  /* unlink associated files */
if (strchr(line+1, '/') || strncmp(line+1, df, 2))
@@ -266,7 +266,7 @@ chk(char *file)
close(fd);
return(0);
}
-   while (getline(cfp)) {
+   while (lpgetline(cfp)) {
if (line[0] == 'P')
break;
}
Index: usr.sbin/lpr/lpc/cmds.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.sbin/lpr/lpc/cmds.c,v
retrieving revision 1.21
diff -a -p -u -r1.21 cmds.c
--- usr.sbin/lpr/lpc/cmds.c 29 Oct 2009 20:11:09 -  1.21
+++ usr.sbin/lpr/lpc/cmds.c 25 Feb 2012 13:33:48 -
@@ -165,7 +165,7 @@ abortpr(int dis)
printf(\tcannot open lock file\n);
goto out;
}
-   if (!getline(fp) || flock(fileno(fp), LOCK_SH|LOCK_NB) == 0) {
+   if (!lpgetline(fp) || flock(fileno(fp), LOCK_SH|LOCK_NB) == 0) {
(void)fclose(fp);   /* unlocks as well */
printf(\tno daemon to abort\n);
goto out;
@@ -1098,7 +1098,7 @@ doarg(char *job)
close(fd);
continue;
}
-   while (getline(fp)  0)
+   while (lpgetline(fp)  0)
if (line[0] == 'P')
break;
(void)fclose(fp);
Index: usr.sbin/lpr/lpd/printjob.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.sbin/lpr/lpd/printjob.c,v
retrieving revision 1.46
diff -a -p -u -r1.46 printjob.c
--- usr.sbin/lpr/lpd/printjob.c 22 Mar 2010 16:50:38 -  1.46
+++ usr.sbin/lpr/lpd/printjob.c 25 Feb 2012 13:33:40 -
@@ -387,12 +387,12 @@ printit(char *file)
 *(after we print it. (Pass 2 only)).
 *  M -- mail to user when done printing
 *
-*  getline reads a line and expands tabs to blanks

Re: NEW: libc getdelim(3) and getline(3)

2012-02-25 Thread Steffen Daode Nurpmeso
diff3(1).

--steffen

Index: usr.bin/diff3/diff3prog.c
===
RCS file: /Users/steffen/arena/code.openbsd/src/usr.bin/diff3/diff3prog.c,v
retrieving revision 1.11
diff -a -p -u -r1.11 diff3prog.c
--- usr.bin/diff3/diff3prog.c   27 Oct 2009 23:59:37 -  1.11
+++ usr.bin/diff3/diff3prog.c   25 Feb 2012 13:44:05 -
@@ -124,7 +124,7 @@ char f1mark[40], f3mark[40];/* markers 
 int duplicate(struct range *, struct range *);
 int edit(struct diff *, int, int);
 char *getchange(FILE *);
-char *getline(FILE *, size_t *);
+char *xgetline(FILE *, size_t *);
 int number(char **);
 int readin(char *, struct diff **);
 int skip(int, int, char *);
@@ -253,7 +253,7 @@ getchange(FILE *b)
 {
char *line;
 
-   while ((line = getline(b, NULL))) {
+   while ((line = xgetline(b, NULL))) {
if (isdigit((unsigned char)line[0]))
return (line);
}
@@ -261,7 +261,7 @@ getchange(FILE *b)
 }
 
 char *
-getline(FILE *b, size_t *n)
+xgetline(FILE *b, size_t *n)
 {
char *cp;
size_t len;
@@ -456,7 +456,7 @@ skip(int i, int from, char *pr)
char *line;
 
for (n = 0; cline[i]  from - 1; n += j) {
-   if ((line = getline(fp[i], j)) == NULL)
+   if ((line = xgetline(fp[i], j)) == NULL)
trouble();
if (pr != NULL)
printf(%s%s, pr, line);



Re: ksh(1): encapsulate hashtable iteration usage

2012-02-19 Thread Steffen Daode Nurpmeso
Otto Moerbeek wrote [2012-02-19 08:49+0100]:
 while I did graduate on a theoretical computer science subject myself

I have no doubt about that. (?)

 I think this alternate hash table stuff is all overkill for ksh.

But a linear array based implementation with INT_MAX/2 is a heavy
thing.  Did you sent this to me only to get some kind of
INT_MAX/2 is probably a bit too large for a limit?  Yes, using
specific hash algorithms for specific array sizes may aid in
making things better?  If so - yes, i think you're right.

And for one i've found the cause of the higher memory consumption.
The problem was simply that the variable names of the test were so
short.

If the variable name is instead XX${i} then the memory
overhead of the current implementation is already larger than that
of the node-based one.  It thus seems as if the single pointer
that the node-based implementation adds to struct tbl causes the
memory allocator to allocate from a larger allocation slot.
So this is the result for the shown longer variable name:

Node (400%/400% load factor):
0m14.72s real   0m14.41s user0m0.25s system
 0m5.81s real0m5.70s user0m0.06s system
 0m5.87s real0m5.76s user0m0.09s system

27781 R+ 95.9  4840  3904 ttyp0 6:29PM0:04.80 obj/ksh tt2.sh

Current (Torek):
0m14.20s real   0m13.71s user 0m0.21s system
 0m5.90s real0m5.85s user 0m0.06s system
 0m5.82s real0m5.64s user 0m0.12s system

17422 R+ 91.0  5052  4316 ttyp0 6:28PM0:02.58 ../ksh/obj/ksh tt2.sh

That looks good!

 Added to that, the ksh code is tricky, so I'd resist big changes.

Tricky is a nice word for this code.

And i'm really sorry about that, as you surely can imagine.

But note i will post the node-based version once it is running as
it should, and maybe you'll give it a try nonetheless, then.
That would be fine.

   -Otto

Yours,

--steffen



Re: ksh(1): encapsulate hashtable iteration usage

2012-02-18 Thread Steffen Daode Nurpmeso
Steffen Daode Nurpmeso wrote [2012-02-15 14:55+0100]:
 the patch below localizes access of struct table internals to
 table.c by using the ktwalk()/ktnext() interface from proto.h
 instead of doing handcrafted table iterations.

Yes it was wrong because it didn't take the flags into account
correctly.
This new version offers an additional field in struct tstate which
defaults to DEFINED but can be overwritten by users to the desired
flags.

 Surely a useful change regardless of possibly turning over to
 a node-based hashmap approach.

I still think so.
This patch also includes the following:

ktinit() shouldn't expect pow2 size requests..

It is an implementation detail that table.array is pow2 spaced,
so let callers simply request the sizes they want, and take care
of the adjustment internally.

The diff can be applied with or without your patch already
included.  It passes this:

  #!/bin/sh
  KSH=obj/ksh
  max=24000
  booogie() {
local i=0
while [[ $i -le $max ]]; do
  eval X${i}=yes
  i=$(($i + 1))
done
i=0
while test $i -lt $max; do
  test $((i  1)) -eq 0  eval unset X${i}
  i=$((i + 1))
done
i=0
while test $i -lt $max; do
  test $((i  1)) -eq 0  eval X${i}=yes
  i=$((i + 1))
done
i=0
while test $i -lt $max; do
  test $((i  1)) -ne 0  eval unset X${i}
  i=$((i + 1))
done
i=0
while test $i -lt $max; do
  test $((i  1)) -eq 0  eval unset X${i}
  i=$((i + 1))
done
  }

  time booogie
  time booogie
  time booogie

So sorry for the first patch, it has been sent too fast!

About the node-based hashmap.
I tell nothing new.
The problem of associative tables is of course the distribution of
the keys, which typically ends up like so:

  tstats(20a601228=var.c:newblock:vars)
* Overview
  - Array capacity :   4096
  - Buckets (K/V pairs):  21067
  - Next grow  :  32768 buckets
* Stats
  - Accesses   : 199655
  - Lookups: 199653
  - Hits with !DEFINED :  21000
  - Direct hits: 106478
  - Bucket resorts :  40545
  - Array resizes  : 10
* Array index statistics
  - Empty indices: 99
  - Indices with multiple buckets:   3759
  - Buckets per index, maximum   : 12
[Don't trust the following two]
  - Buckets per index  average   :  5 (ideal distribution)
  - Buckets per index, average   :  5 (real distribution)
* Index overview (index / buckets)
0/ 5 1/ 4 2/ 4 3/ 4 4/ 4 5/ 6 6/ 7 
7/ 7 8/ 7 9/ 710/ 711/ 612/ 513/ 4 
   14/ 315/ 116/ 017/ 018/ 019/ 120/ 2 
  [.]
  105/ 8   106/ 8   107/ 8   108/ 7   109/ 6   110/ 5   111/ 3 
  112/ 2   113/ 1   114/ 0   115/ 1   116/ 1   117/ 2   118/ 4 
  119/ 5   120/ 6   121/ 7   122/ 8   123/ 9   124/ 9   125/ 8 
  [.]
  910/ 1   911/ 0   912/ 0   913/ 2   914/ 3   915/ 4   916/ 5 
  917/ 7   918/ 9   919/10   920/10   921/10   922/10   923/ 8 
  924/ 7   925/ 6   926/ 5   927/ 3   928/ 2   929/ 2   930/ 3 

with even worser distributions for arrays of 1024 and 2048 slots;
maybe a mathematician can tell you why this is so, i'm not the one.

The problems of using non-node based, i.e., linear hashtables is
thus of course that heaps can unite; e.g., the indexes 919 - 922
contain 40 struct tbl* entries which in the worst case had to be
all checked to find the desired one.
And the only way to ease that problem is to reduce the load
factor, i.e., to place fewer entries in the array before that is
resized, but which necessarily enlargens the array.
And that enlargement must be power-of-two spaced:

  RESIZE: old-nsize=8192, new=16384 = 131072;  new-free:11468
  RESIZE: old-nsize=16384, new=32768 = 262144;  new-free:22937

Node-based hashmaps, on the other hand, split up such heaps;
you access the array once and then do a list walk.
In the above example the worst-case is thus 10 node walks.
Bucket resorting can also be a good thing, as you can see above
(without resorting there are 8910 direct hits), but it surely
can result in the opposite, dependent on the actual use-case.

The best thing about node-based hashmaps is however that you can
reduce the load factor; in the above log extract the map has
a load factor of 800% (32768 / 4096).  E.g., whereas the linear
one requires 261144 bytes of continuous memory, the node one
only requires 32768.

Well and after these well-known basics some more numbers.
If i run the above test in a VM with (the below patch and Chris
Toreks hash plus applied) i get these:

2. 0m6.63s real 0m6.51s user 0m0.07s system
3. 0m6.65s real 0m6.63s user 0m0.02s system

For the node-based with 2==400% to 256 slots, then 3==800% load:

2. 0m7.46s real 0m7.32s user 0m0.11s system
3

ksh(1): encapsulate hashtable iteration usage

2012-02-15 Thread Steffen Daode Nurpmeso
Hey all,

the patch below localizes access of struct table internals to
table.c by using the ktwalk()/ktnext() interface from proto.h
instead of doing handcrafted table iterations.
Surely a useful change regardless of possibly turning over to
a node-based hashmap approach.

--steffen

Index: bin/ksh/var.c
===
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.34
diff -a -p -u -r1.34 var.c
--- bin/ksh/var.c   15 Oct 2007 02:16:35 -  1.34
+++ bin/ksh/var.c   15 Feb 2012 13:29:18 -
@@ -60,16 +60,17 @@ void
 popblock(void)
 {
struct block *l = e-loc;
-   struct tbl *vp, **vpp = l-vars.tbls, *vq;
-   int i;
+   struct tbl *vp;
+   struct tstate ts;
 
e-loc = l-next;   /* pop block */
-   for (i = l-vars.size; --i = 0; )
-   if ((vp = *vpp++) != NULL  (vp-flagSPECIAL)) {
-   if ((vq = global(vp-name))-flag  ISSET)
-   setspec(vq);
+   ktwalk(ts, l-vars);
+   while ((vp = ktnext(ts)))
+   if (vp-flag  SPECIAL) {
+   if ((vp = global(vp-name))-flag  ISSET)
+   setspec(vp);
else
-   unsetspec(vq);
+   unsetspec(vp);
}
if (l-flags  BF_DOGETOPTS)
user_opt = l-getopts_state;
@@ -832,14 +833,14 @@ makenv(void)
 {
struct block *l = e-loc;
XPtrV env;
-   struct tbl *vp, **vpp;
-   int i;
+   struct tbl *vp;
+   struct tstate ts;
 
XPinit(env, 64);
-   for (l = e-loc; l != NULL; l = l-next)
-   for (vpp = l-vars.tbls, i = l-vars.size; --i = 0; )
-   if ((vp = *vpp++) != NULL 
-   (vp-flag(ISSET|EXPORT)) == (ISSET|EXPORT)) {
+   for (l = e-loc; l != NULL; l = l-next) {
+   ktwalk(ts, l-vars);
+   while ((vp = ktnext(ts)))
+   if ((vp-flag  (ISSET|EXPORT)) == (ISSET|EXPORT)) {
struct block *l2;
struct tbl *vp2;
unsigned int h = hash(vp-name);
@@ -860,6 +861,7 @@ makenv(void)
}
XPput(env, vp-val.s);
}
+   }
XPput(env, NULL);
return (char **) XPclose(env);
 }



Update hash macro in sys/hash.h; gcc(1) ambiguity

2012-02-14 Thread Steffen Daode Nurpmeso
It turns out that sys/hash.h also uses Chris Toreks hash algorithm
in the same 1:1 way that was present in Berkeley DB a decade ago.
So maybe i prefer leaving optimization up to the compiler should
be applied here, too.  The patch does this.

And - you know, i never made it into that gcc(1) code jungle, but
maybe someone with experience simply needs to toggle a switch for
better behaviour - the following seems strange to me:

#include stdio.h
int main(void) {
int i = 3;
printf(i = %d\n, i);
#ifdef T1
for (int x = 1; x  i; ++x)
#else
int x = 1;
#endif
printf(x = %d\n, x);
return 0;
}

?0%1[steffen@obsd tmp]$ /usr/bin/gcc -W -Wall -pedantic -o t -c t.c  
t.c: In function 'main':
t.c:9: warning: ISO C90 forbids mixed declarations and code
?0%1[steffen@obsd tmp]$ /usr/bin/gcc -DT1 -W -Wall -pedantic -o t -c t.c 
t.c: In function 'main':
t.c:7: error: 'for' loop initial declaration used outside C99 mode
?1%1[steffen@obsd tmp]$

E.g., clang(1) does this (also for -std=c90):

?0%0[steffen@sherwood tmp]$ clang -DT1 -W -Wall -pedantic -ansi -o t -c t.c
t.c:7:14: warning: variable declaration in for loop is a C99-specific 
feature [-pedantic]
for (int x = 1; x  i; ++x)
 ^
1 warning generated.
?0%0[steffen@sherwood tmp]$ clang  -W -Wall -pedantic -ansi -o t -c t.c
t.c:9:13: warning: ISO C90 forbids mixing declarations and code 
[-Wdeclaration-after-statement]
int x = 1;
^
1 warning generated.
?0%0[steffen@sherwood tmp]$

--steffen

Index: sys/sys/hash.h
===
RCS file: /Users/steffen/arena/code.openbsd/src/sys/sys/hash.h,v
retrieving revision 1.5
diff -a -p -u -r1.5 hash.h
--- sys/sys/hash.h  24 Sep 2010 13:24:53 -  1.5
+++ sys/sys/hash.h  14 Feb 2012 17:05:43 -
@@ -40,7 +40,7 @@
 /* Convenience */
 #ifndefHASHINIT
 #defineHASHINIT5381
-#defineHASHSTEP(x,c)   (((x  5) + x) + (c))
+#defineHASHSTEP(x,c)   ((x)*33 + (c))
 #endif
 
 /*



Re: Keycode print mode (-r) for wsconsctl(8)

2012-02-01 Thread Steffen Daode Nurpmeso
For one: Tobias Stoeckmann already stated more yesterday,
including that exit(3) in a signal handler is a bad thing.
(It surely should have been _exit(2).)

Version 3 (1) doesn't trash errno no more, (2) is hopefully less
messy and (3) does no longer introduce a new file, so also more
easyness for cvs(1).
It fixes a compiler warning in tab_by_name().

Beside that: :c
Thank you very much.

--steffen

Index: src/sbin/wsconsctl/wsconsctl.8
===
RCS file: /cvs/src/sbin/wsconsctl/wsconsctl.8,v
retrieving revision 1.23
diff -a -p -u -r1.23 wsconsctl.8
--- wsconsctl.8 4 Aug 2008 07:32:51 -   1.23
+++ wsconsctl.8 1 Feb 2012 14:41:11 -
@@ -49,6 +49,8 @@
 .Op Fl n
 .Op Fl f Ar file
 .Ar name Ns += Ns Ar value ...
+.Nm wsconsctl
+.Fl r
 .Sh DESCRIPTION
 The
 .Nm
@@ -81,6 +83,12 @@ symbol.
 See the
 .Sx EXAMPLES
 section for more details.
+.It Fl r
+Enters a raw interactive mode in which keypresses are displayed
+as their keycodes.
+This mode always operates on standard input
+and does not work on pseudo terminals.
+After five seconds of inactivity the program terminates.
 .El
 .Pp
 The
Index: src/sbin/wsconsctl/wsconsctl.c
===
RCS file: /cvs/src/sbin/wsconsctl/wsconsctl.c,v
retrieving revision 1.26
diff -a -p -u -r1.26 wsconsctl.c
--- wsconsctl.c 20 Aug 2010 00:20:55 -  1.26
+++ wsconsctl.c 1 Feb 2012 14:48:27 -
@@ -30,12 +30,18 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include dev/wscons/wsconsio.h
+#include dev/wscons/wsksymdef.h
 #include fcntl.h
 #include err.h
 #include errno.h
+#include signal.h
 #include string.h
 #include stdio.h
 #include stdlib.h
+#include sys/ioctl.h
+#include sys/time.h
+#include termios.h
 #include unistd.h
 #include wsconsctl.h
 
@@ -45,9 +51,7 @@ extern struct field keyboard_field_tab[]
 extern struct field mouse_field_tab[];
 extern struct field display_field_tab[];
 
-void   usage(void);
-
-struct vartypesw {
+static struct vartypesw {
const   char *name;
struct field *field_tab;
void(*getval)(int);
@@ -63,24 +67,15 @@ struct vartypesw {
{ NULL }
 };
 
-struct vartypesw *tab_by_name(const char *, int *);
-
-void
-usage()
-{
-   fprintf(stderr,
-   usage: %s [-an]\n
-  %s [-n] [-f file] name ...\n
-  %s [-n] [-f file] name=value ...\n
-  %s [-n] [-f file] name+=value ...\n,
-   __progname, __progname, __progname, __progname);
-   exit(1);
-}
+static voidusage(void);
+static struct vartypesw*tab_by_name(const char *, int *);
+static int raw_mode(void);
 
 int
 main(int argc, char *argv[])
 {
-   int i, ch, error = 0, aflag = 0, do_merge, putval, devidx, devfd;
+   int i, ch, error = 0, aflag = 0, rflag = 0,
+   do_merge, putval, devidx, devfd;
struct vartypesw *sw = NULL;
char *getsep = =, *setsep =  - , *p;
char *wdev = NULL;
@@ -88,7 +83,7 @@ main(int argc, char *argv[])
struct field *f;
char devname[20];
 
-   while ((ch = getopt(argc, argv, af:nw)) != -1) {
+   while ((ch = getopt(argc, argv, af:nrw)) != -1) {
switch(ch) {
case 'a':
aflag = 1;
@@ -99,6 +94,9 @@ main(int argc, char *argv[])
case 'n':
getsep = setsep = NULL;
break;
+   case 'r':
+   rflag = 1;
+   break;
case 'w':
/* compat */
break;
@@ -110,12 +108,17 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
 
-   if (argc  0  aflag != 0)
-   errx(1, excess arguments after -a);
-   if (argc == 0)
-   aflag = 1;
+   if (argc  0  (aflag != 0 || rflag != 0))
+   errx(1, excess arguments after -%c, (aflag ? 'a' : 'r'));
+   if (rflag == 0) {
+   if (argc == 0)
+   aflag = 1;
+   } else if (aflag != 0 || wdev != NULL || getsep == NULL)
+   errx(1, -r is mutual exclusive with other options);
 
-   if (aflag != 0) {
+   if (rflag != 0)
+   error = raw_mode();
+   else if (aflag != 0) {
for (sw = typesw; sw-name; sw++) {
for (devidx = 0;; devidx++) {
device = (*sw-nextdev)(devidx);
@@ -280,7 +283,20 @@ main(int argc, char *argv[])
exit(error);
 }
 
-struct vartypesw *
+static void
+usage()
+{
+   fprintf(stderr,
+   usage: %s [-an]\n
+  %s [-n] [-f file] name ...\n
+  %s [-n] [-f file] name=value ...\n
+  %s [-n] [-f file] name+=value ...\n
+  %s -r\n,
+   __progname, __progname, __progname, __progname, __progname);
+   exit(1);
+}
+

Re: Keycode print mode (-r) for wsconsctl(8)

2012-01-31 Thread Steffen Daode Nurpmeso
Tobias Stoeckmann suggested off-list to keep explicit state of
wether rawmode is enabled or not, so that the signal handler
doesn't perform useless (and wrong, strictly speaking) work.
He also suggested to integrate keycode.c into wsconsctl.c (if
i understood correctly ;), but i was unsure about that.

And the usage string for -r erroneously and unfortunately
contained '-f [xy]', which this diff also fixes.
I'll watch out and see if i find my heart drops somewhere.

Thanks again.

--steffen

Index: Makefile
===
RCS file: /Users/steffen/arena/code.openbsd/src/sbin/wsconsctl/Makefile,v
retrieving revision 1.34
diff -a -p -u -r1.34 Makefile
--- Makefile30 Jan 2010 20:48:50 -  1.34
+++ Makefile30 Jan 2012 15:17:46 -
@@ -12,7 +12,7 @@
 ${MACHINE} == zaurus
 
 PROG=  wsconsctl
-SRCS=  display.c keyboard.c keysym.c map_parse.y map_scan.l \
+SRCS=  display.c keyboard.c keycode.c keysym.c map_parse.y map_scan.l \
mouse.c util.c wsconsctl.c
 
 CPPFLAGS+= -I${.CURDIR} -I.
Index: keycode.c
===
RCS file: keycode.c
diff -N keycode.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ keycode.c   31 Jan 2012 13:46:22 -
@@ -0,0 +1,139 @@
+/* $OpenBSD$   */
+
+/*
+ * Copyright (c) 2012 Steffen Daode Nurpmeso.
+ * All rights reserved.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include dev/wscons/wsconsio.h
+#include dev/wscons/wsksymdef.h
+#include err.h
+#include errno.h
+#include signal.h
+#include stdio.h
+#include stdlib.h
+#include sys/ioctl.h
+#include sys/time.h
+#include termios.h
+#include unistd.h
+#include wsconsctl.h
+
+static int is_rawmode;
+static struct termios  tios_orig, tios_raw;
+
+static voidonsig(int sig);
+static voidraw_on(void), raw_off(void);
+
+static void
+onsig(int sig)
+{
+   if (is_rawmode)
+   raw_off();
+   exit(sig != SIGALRM);
+}
+
+static void
+raw_on(void)
+{
+   int arg = WSKBD_RAW;
+
+   if (tcsetattr(STDIN_FILENO, TCSANOW, tios_raw)  0)
+   err(1, Can't set terminal attributes);
+   if (ioctl(STDIN_FILENO, WSKBDIO_SETMODE, arg)  0) {
+   arg = errno;
+   (void)tcsetattr(STDIN_FILENO, TCSANOW, tios_orig);
+   errno = arg;
+   err(1, ((arg == ENOTTY)
+   ? This mode won't work on pseudo terminals
+   : Can't put keyboard in raw mode (the 
+ WSDISPLAY_COMPAT_RAWKBD kernel option is required)));
+   }
+
+   is_rawmode = 1;
+   return;
+}
+
+static void
+raw_off(void)
+{
+   int arg = WSKBD_TRANSLATED;
+
+   (void)ioctl(STDIN_FILENO, WSKBDIO_SETMODE, arg);
+   (void)tcsetattr(STDIN_FILENO, TCSANOW, tios_orig);
+
+   is_rawmode = 0;
+   return;
+}
+
+void
+keycode_mode(void)
+{
+   ssize_t i, skip;
+   struct sigaction sa;
+   struct itimerval it;
+   unsigned char *cursor, buffer[64];
+
+   if (tcgetattr(STDIN_FILENO, tios_orig)  0)
+   err(1, Can't query terminal attributes);
+   tios_raw = tios_orig;
+   (void)cfmakeraw(tios_raw);
+
+   sa.sa_handler = onsig;
+   sa.sa_flags = 0;
+   (void)sigfillset(sa.sa_mask);
+   for (i = 0; i  NSIG; ++i)
+   if (sigaction((int)i + 1, sa, NULL)  0  i == SIGALRM)
+   err(1, Can't install SIGALRM signal handler);
+
+   printf(You may now use the keyboard.\n
+   After five seconds of inactivity the program terminates\n);
+   for (i = skip = 0;;) {
+   it.it_value.tv_sec = 5;
+   it.it_value.tv_usec = 0;
+   it.it_interval.tv_sec = it.it_interval.tv_usec = 0;
+   if (setitimer(ITIMER_REAL, it, NULL)  0)
+   err(3, Can't install wakeup timer);
+
+   raw_on();
+   i = read(STDIN_FILENO, buffer + skip, sizeof(buffer) - skip);
+   raw_off();
+
+   it.it_value.tv_sec = it.it_value.tv_usec = 0;
+   (void)setitimer(ITIMER_REAL, it, NULL);
+
+   if (i = 0)
+   exit(1);
+   i += skip;
+
+   for (cursor = buffer; --i = 0

Keycode print mode (-r) for wsconsctl(8)

2012-01-30 Thread Steffen Daode Nurpmeso
It doesn't offer any information from the generated keysym.h or
anything, but only prints the plain keycode.
If it's of any use for a user to see 'keycode 83' and not being
able to say 'keyboard.map+=keycode 83=Home' is a different
story.

--steffen

Index: sbin/wsconsctl/Makefile
===
RCS file: /cvs/src/sbin/wsconsctl/Makefile,v
retrieving revision 1.34
diff -a -p -u -r1.34 Makefile
--- sbin/wsconsctl/Makefile 30 Jan 2010 20:48:50 -  1.34
+++ sbin/wsconsctl/Makefile 30 Jan 2012 15:17:46 -
@@ -12,7 +12,7 @@
 ${MACHINE} == zaurus
 
 PROG=  wsconsctl
-SRCS=  display.c keyboard.c keysym.c map_parse.y map_scan.l \
+SRCS=  display.c keyboard.c keycode.c keysym.c map_parse.y map_scan.l \
mouse.c util.c wsconsctl.c
 
 CPPFLAGS+= -I${.CURDIR} -I.
Index: sbin/wsconsctl/keycode.c
===
RCS file: sbin/wsconsctl/keycode.c
diff -N sbin/wsconsctl/keycode.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ sbin/wsconsctl/keycode.c30 Jan 2012 15:18:12 -
@@ -0,0 +1,133 @@
+/* $OpenBSD$   */
+
+/*
+ * Copyright (c) 2012 Steffen Daode Nurpmeso.
+ * All rights reserved.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include dev/wscons/wsconsio.h
+#include dev/wscons/wsksymdef.h
+#include err.h
+#include errno.h
+#include signal.h
+#include stdio.h
+#include stdlib.h
+#include sys/ioctl.h
+#include sys/time.h
+#include termios.h
+#include unistd.h
+#include wsconsctl.h
+
+static struct termios  tios_orig, tios_raw;
+
+static voidonsig(int sig);
+static voidraw_on(void), raw_off(void);
+
+static void
+onsig(int sig)
+{
+   raw_off();
+   exit(sig != SIGALRM);
+}
+
+static void
+raw_on(void)
+{
+   int arg = WSKBD_RAW;
+
+   if (tcsetattr(STDIN_FILENO, TCSANOW, tios_raw)  0)
+   err(1, Can't set terminal attributes);
+   if (ioctl(STDIN_FILENO, WSKBDIO_SETMODE, arg)  0) {
+   arg = errno;
+   (void)tcsetattr(STDIN_FILENO, TCSANOW, tios_orig);
+   errno = arg;
+   err(1, ((arg == ENOTTY)
+   ? This mode won't work on pseudo terminals
+   : Can't put keyboard in raw mode (the 
+ WSDISPLAY_COMPAT_RAWKBD kernel option is required)));
+   }
+   return;
+}
+
+static void
+raw_off(void)
+{
+   int arg = WSKBD_TRANSLATED;
+
+   (void)ioctl(STDIN_FILENO, WSKBDIO_SETMODE, arg);
+   (void)tcsetattr(STDIN_FILENO, TCSANOW, tios_orig);
+   return;
+}
+
+void
+keycode_mode(void)
+{
+   ssize_t i, skip;
+   struct sigaction sa;
+   struct itimerval it;
+   unsigned char *cursor, buffer[64];
+
+   if (tcgetattr(STDIN_FILENO, tios_orig)  0)
+   err(1, Can't query terminal attributes);
+   tios_raw = tios_orig;
+   (void)cfmakeraw(tios_raw);
+
+   sa.sa_handler = onsig;
+   sa.sa_flags = 0;
+   (void)sigfillset(sa.sa_mask);
+   for (i = 0; i  NSIG; ++i)
+   if (sigaction((int)i + 1, sa, NULL)  0  i == SIGALRM)
+   err(1, Can't install SIGALRM signal handler);
+
+   printf(You may now use the keyboard.\n
+   After five seconds of inactivity the program terminates\n);
+   for (i = skip = 0;;) {
+   it.it_value.tv_sec = 5;
+   it.it_value.tv_usec = 0;
+   it.it_interval.tv_sec = it.it_interval.tv_usec = 0;
+   if (setitimer(ITIMER_REAL, it, NULL)  0)
+   err(3, Can't install wakeup timer);
+
+   raw_on();
+   i = read(STDIN_FILENO, buffer + skip, sizeof(buffer) - skip);
+   raw_off();
+
+   it.it_value.tv_sec = it.it_value.tv_usec = 0;
+   (void)setitimer(ITIMER_REAL, it, NULL);
+
+   if (i = 0)
+   exit(1);
+   i += skip;
+
+   for (cursor = buffer; --i = 0;) {
+   unsigned int k = *cursor++;
+   if ((k  0xF0) == 0xE0 || (k  0xF8) == 0xF0) {
+   if (--i  0) {
+   buffer[0] = (unsigned char)k;
+   skip = 1

wscons: rename KS_GROUP_Ascii to _Plain

2012-01-29 Thread Steffen Daode Nurpmeso
Hey all interested,

while writing a small wscons(4) keycode utility i asked myself
why KS_GROUP_Ascii is named the way it is, since the (lower) byte
carries more information, at least eventually.

I looked around a bit, and found out that NetBSD did change the
name to KS_GROUP_Plain, which is in my view less misleading than
the current naming scheme.
So the diff below changes all occurrences in todays src/ (no
change at all in xenocara) to _Plain.

And i've also missed a macro which tells an application wether
a second byte is necessary to complete the current raw keycode.
I.e., currently i hardcode

((rc  0xF0) == 0xE0 || (rc  0xF8) == 0xF0)

but can/could use KS_GROUP()/KS_VALUE() to separate the components
after i finally have read the complete keysym.
This is not part of the diff, however.
Ciao,

--steffen

Index: sbin/wsconsctl/keysym.c
===
RCS file: /cvs/src/sbin/wsconsctl/keysym.c,v
retrieving revision 1.6
diff -a -p -u -r1.6 keysym.c
--- sbin/wsconsctl/keysym.c 28 Jun 2010 20:40:39 -  1.6
+++ sbin/wsconsctl/keysym.c 29 Jan 2012 17:37:04 -
@@ -229,7 +229,7 @@ ksym_upcase(keysym_t ksym)
if (ksym = KS_f1  ksym = KS_f20)
return(KS_F1 - KS_f1 + ksym);
 
-   if (KS_GROUP(ksym) == KS_GROUP_Ascii  ksym = 0xff 
+   if (KS_GROUP(ksym) == KS_GROUP_Plain  ksym = 0xff 
latin1_to_upper[ksym] != 0x00)
return(latin1_to_upper[ksym]);
 
Index: sys/dev/wscons/wsdisplay.c
===
RCS file: /cvs/src/sys/dev/wscons/wsdisplay.c,v
retrieving revision 1.105
diff -a -p -u -r1.105 wsdisplay.c
--- sys/dev/wscons/wsdisplay.c  3 Jul 2011 18:11:21 -   1.105
+++ sys/dev/wscons/wsdisplay.c  29 Jan 2012 17:38:44 -
@@ -1594,7 +1594,7 @@ wsdisplay_kbdinput(struct device *dev, k
 
tp = scr-scr_tty;
for (; num  0; num--, ks++) {
-   if (KS_GROUP(*ks) == KS_GROUP_Ascii)
+   if (KS_GROUP(*ks) == KS_GROUP_Plain)
(*linesw[tp-t_line].l_rint)(KS_VALUE(*ks), tp);
else {
count = (*scr-scr_dconf-wsemul-translate)
Index: sys/dev/wscons/wskbd.c
===
RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
retrieving revision 1.70
diff -a -p -u -r1.70 wskbd.c
--- sys/dev/wscons/wskbd.c  9 Nov 2011 14:27:52 -   1.70
+++ sys/dev/wscons/wskbd.c  29 Jan 2012 17:38:16 -
@@ -1265,7 +1265,7 @@ wskbd_cngetc(dev_t dev)
for(;;) {
if (num--  0) {
ks = wskbd_console_data.t_symbols[pos++];
-   if (KS_GROUP(ks) == KS_GROUP_Ascii)
+   if (KS_GROUP(ks) == KS_GROUP_Plain)
return (KS_VALUE(ks));  
} else {
(*wskbd_console_data.t_consops-getc)
@@ -1667,7 +1667,7 @@ wskbd_translate(struct wskbd_internal *i
res = KS_voidSymbol;
 
switch (KS_GROUP(ksym)) {
-   case KS_GROUP_Ascii:
+   case KS_GROUP_Plain:
case KS_GROUP_Keypad:
case KS_GROUP_Function:
res = ksym;
@@ -1716,7 +1716,7 @@ wskbd_translate(struct wskbd_internal *i
}
 
/* We are done, return the symbol */
-   if (KS_GROUP(res) == KS_GROUP_Ascii) {
+   if (KS_GROUP(res) == KS_GROUP_Plain) {
if (MOD_ONESET(id, MOD_ANYCONTROL)) {
if ((res = KS_at  res = KS_z) || res == KS_space)
res = res  0x1f;
Index: sys/dev/wscons/wskbdutil.c
===
RCS file: /cvs/src/sys/dev/wscons/wskbdutil.c,v
retrieving revision 1.9
diff -a -p -u -r1.9 wskbdutil.c
--- sys/dev/wscons/wskbdutil.c  28 Aug 2010 12:48:14 -  1.9
+++ sys/dev/wscons/wskbdutil.c  29 Jan 2012 17:37:44 -
@@ -268,7 +268,7 @@ ksym_upcase(keysym_t ksym)
if (ksym = KS_f1  ksym = KS_f20)
return(KS_F1 - KS_f1 + ksym);
 
-   if (KS_GROUP(ksym) == KS_GROUP_Ascii  ksym = 0xff 
+   if (KS_GROUP(ksym) == KS_GROUP_Plain  ksym = 0xff 
latin1_to_upper[ksym] != 0x00)
return(latin1_to_upper[ksym]);
 
Index: sys/dev/wscons/wsksymdef.h
===
RCS file: /cvs/src/sys/dev/wscons/wsksymdef.h,v
retrieving revision 1.34
diff -a -p -u -r1.34 wsksymdef.h
--- sys/dev/wscons/wsksymdef.h  5 Apr 2011 19:12:13 -   1.34
+++ sys/dev/wscons/wsksymdef.h  29 Jan 2012 17:45:41 -
@@ -684,7 +684,7 @@
 #define KS_GROUP_Command   0xf400
 #define KS_GROUP_Internal  0xf500
 #define KS_GROUP_Dead  0xf801  /* not encoded in keysym */
-#define KS_GROUP_Ascii 0xf802  /* not encoded in keysym */
+#define KS_GROUP_Plain 0xf802  /* not encoded in keysym */
 #define 

Re: vmmap replacement -- please test

2012-01-14 Thread Steffen Daode Nurpmeso
Ariane van der Steldt wrote [2012-01-14 08:42+0100]:
 As far as I'm concerned, this diff will be commited once we unlock after
 release (in a coordinated manner ofcourse, since this is uvm we're
 talking about).
 It's about time too, ofcourse: 64 revisions of the same diff is alot. :D
 -- 
 Ariane

I too cannot report anything to bring you forward (amd64).

--steffen



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);